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

4.2 use_session option prevents react admin areas loading #607

Closed
1 task done
blueo opened this issue Aug 21, 2018 · 5 comments
Closed
1 task done

4.2 use_session option prevents react admin areas loading #607

blueo opened this issue Aug 21, 2018 · 5 comments

Comments

@blueo
Copy link
Contributor

blueo commented Aug 21, 2018

4.2 has the option to keep using sessions to persist the current stage using this config:

SilverStripe\Versioned\Versioned:
  use_session: true

When this is set, the react-based areas such as asset admin do not load.

This is caused by getCombinedClientConfig returning the url for the admin section with stage query params attached (eg /assets/admin?stage=Stage). This means that admin's BootRoutes.js fails to match this route as a react-based route (matchesReactRoute line 71) and instead starts the legacy router.

I'm not 100% sure but I think this stems from LeftAndMain's init method, as it tries to set the reading mode to the same as the default reading mode to avoid having the stage param in the cms. When using session, this process is interrupted or changed before the config is generated and as a result the url is incorrect.

To reproduce in 4.2 installation:

  • set config value above, flush
  • visit a front-end page in draft mode (eg /?stage=Stage)
  • then visit /admin/assets, note that the window.ss.config has urls with query parameters set.

PRs

@maxime-rainville
Copy link
Contributor

maxime-rainville commented Aug 24, 2018

Thanks for the bug report and the detailed instruction. That was really easy to follow.

I've confirmed in 4.x-dev. That got introduce by silverstripe/silverstripe-versioned#45

@chillu I'm setting the priority to high as this makes the asset-admin area unusable after you've access a draft page.

Looks like this also broke versionned-admin silverstripe/silverstripe-versioned-admin#51. Might be worthwhile doing more testing use_session: true to see what else blows up.

@maxime-rainville
Copy link
Contributor

It breaks Campaign-Admin as well.

@robbieaverill
Copy link
Contributor

Right - so it's breaking versioned-admin as well, essentially everything that uses this API

@maxime-rainville
Copy link
Contributor

I'm thinking of updating VersionedStateExtension, like this. Can anyone think of some dirty side effect of this?

    public function updateLink(&$link)
    {

        // Skip if current mode matches default mode
        // See LeftAndMain::init() for example of this being overridden.
        $readingMode = $this->getReadingmode();
        if ($readingMode === Versioned::get_default_reading_mode()) {
            return;
        }

        // Determine if query args are supported for the current mode
        $queryargs = ReadingMode::toQueryString($readingMode);
        if (!$queryargs) {
            return;
        }

        // Don't touch Admin/CMS links
        if (class_exists(LeftAndMain::class) && $this->getOwner() instanceof LeftAndMain) {
            return false;
        }

        // Decorate
        $link = Controller::join_links(
            $link,
            '?' . http_build_query($queryargs)
        );
    }

@blueo
Copy link
Contributor Author

blueo commented Sep 6, 2018

Just a usage note that is related. We created a link to asset admin from a dev task (using Image::get()->byID($id)->CMSEditLink()). This appends a stage param and even though we now have the above setting turned off, this url will have the same effect.
in short:

  • ss 4.2.1 with use_session: false
  • link to asset admin with stage param (eg /admin/assets/show/1/edit/36?stage=Stage)
  • admin will not load. Removing stage param allows it to load.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants