Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Add hook to alter admin url #1882

Open
jan-clockworkwp opened this issue May 2, 2024 · 4 comments
Open

[Feature] Add hook to alter admin url #1882

jan-clockworkwp opened this issue May 2, 2024 · 4 comments
Assignees
Labels
good first issue Issue that doesn't require previous experience with codebase help wanted type: feature New functionality being added

Comments

@jan-clockworkwp
Copy link

Currently in @faustwp/core: 3.0.1 package, there is a way to hook into wpUrl method. But there is no way to hook into the adminUrl, and as far as there is a way to override SiteName link in the custom toolbar with snippet like this:

hooks.addFilter(
      'toolbarNodes',
      'faust',
      (toolbarNodes: FaustToolbarNodes, context: FaustToolbarContext) => {
        const adminUrl = getAdminUrl().replace('/wp-admin', '/wp/wp-admin');
        const customToolbarNodes: FaustToolbarNodes = [
          {
            id: 'site-name',
            location: 'primary',
            component: <SiteName url={adminUrl} />,
          },
        ];

        // Removing default site name node to be able to override admin url
        toolbarNodes = [
          ...toolbarNodes.filter((node) => node.id !== 'site-name'),
        ];

        return [...customToolbarNodes, ...toolbarNodes];
      },
    );

there is no way to override rest of the toolbar items to render proper admin urls.

Would be great if we could do something like:

// @note: wpAdminUrl filter is not currently part of @faustwp/core package
hooks.addFilter('wpAdminUrl', 'faust', (url: string) => {
  return url.replace('/wp-admin', '/wp/wp-admin'); // or whatever admin url modification is needed
});

There is a similar request to this one, but it is related to WordPress faustwp plugin #1872 (comment)

@theodesp theodesp added type: feature New functionality being added good first issue Issue that doesn't require previous experience with codebase help wanted labels May 3, 2024
@theodesp theodesp self-assigned this May 29, 2024
@theodesp theodesp mentioned this issue May 30, 2024
3 tasks
@ChrisWiegman
Copy link
Contributor

While @theodesp 's PR, which would solve this in a JavaScript filter for wp-admin in Faust users is excellent, I've asked us both to step back on that for the moment. The issue is that there are more paths than just wp-admin that may need to be accessed directly. As I reviewed his PR I was actually working on #1872 which addresses a different symptom of the same issue with a PHP filter in WordPress. Both would work, but doesn't solve the issue.

The approach we're looking at instead is to expose this via WP GraphQL and I've opened wp-graphql/wp-graphql#3145 to discuss that. I believe this would best help you (so you don't have to ask us for another filter if you have to access something else in the WordPress filesystem) as well as all Faust users and beyond.

@justlevine
Copy link
Contributor

justlevine commented May 31, 2024

On review, it seems the issue comes from the fact that getAdminUrl() makes two incorrect assumptions over in

export function getAdminUrl(path = ''): string {

  1. It conflates the site_url with the home_url (like many parts of Fausts frontend/backend).
  2. It assumes that the admin URI is wp-admin, when it's theoretically possible to move it with some filters and server rewrite rules.

As with #1872 (comment), I believe the ideal pattern is inheritable constants, and shouldn't require an extra GraphQL request to resolve.

(Fun fact #1360 was actually designed to workaround this exact issue, since /graphql is located on site_url but Faust was forcing it on the home url. There are several other examples of this in both the frontend and in replacements.php)

@ChrisWiegman please confirm/correct my assessment 🙏

@jan-clockworkwp
Copy link
Author

@justlevine that is absolutely right, and it took me a while to digest what is going on with the urls in FaustWP plugin and where is the issue coming from, only after realising that site_url() WordPress native method, that is widely used in FaustWp plugin, specially in includes/replacement, can be different (suffixed, based on your setup [Bedrock /wp]) to home_url(), causing that no href, src and url replacement actually work if site_url() is not the same as home_url(), as all Faust plugin rewrites rely on the site_url() but internal links, media and other resources are relying on the actual WordPress front end base url that is equal to home_url(). Probably there are other areas where this incorrect assumption is causing issues as @justlevine pointed out in his comment.
As @ChrisWiegman is not with WPE anymore according to #1872 (comment), @theodesp is there anyone who could take over this kinda feature/bug request? Is there anything I can to do help this one to be merged into the plugin core, and would potentially be solving as well #1872 We are slowly getting closer to production release date and we would love to not go to production with FaustWP plugin patched all over the places. Many thanks for any input.

@jan-clockworkwp
Copy link
Author

@theodesp is there any progress on this one? Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that doesn't require previous experience with codebase help wanted type: feature New functionality being added
Projects
None yet
Development

No branches or pull requests

4 participants