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

Update tests for custom admin path #4375

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IgorKhromov
Copy link

Now with 'admin path' options, list loading is infinity and broken, because of hardcoded keystone path in query params sagas.

@tmvanetten
Copy link
Contributor

@IgorKhromov. I pulled in your changes and it does not resolve the issue. Looks like there is still something hardcoded somewhere.

@tmvanetten
Copy link
Contributor

Anyone else have insight on the custom admin path not working in beta-5?

@tmvanetten
Copy link
Contributor

Any movement planned for this one?

@tmvanetten
Copy link
Contributor

@Noviny this one is solved in #4586

@crazyx13th
Copy link

crazyx13th commented Mar 29, 2018

I think the small problem with your fix is:

  • i set keystone.set('admin path', 'test123');
  • inside queryParamsSagas.js > * evalQueryParams() I insert
    console.log('Keystone.adminPath:', Keystone.adminPath);

output: /test123

your script-line:

if (pathname !== `/${Keystone.adminPath}/${currentList.id}`) return;

is wrong, because result has "double-slash"

if (pathname !== `//test123/${currentList.id}`) return;

thx!

solution:

(sould work,... I hope)
/admin/client/App/sagas/queryParamsSagas.jsqueryParamsSagas.js, line 71:

if (pathname !== `${Keystone.adminPath}/${currentList.id}`) return;

(not testet)
/admin/client/App/sagas/test/queryParamsSagas.test.js, line 69:

const pathname = `${Keystone.adminPath}/${currentList.id}`;

@crazyx13th
Copy link

grafik
ohh, now I got it, sorry, but there isn't a release with this fix, thx!

@stennie stennie added the bug label May 29, 2018
@dev-dem
Copy link

dev-dem commented Jun 18, 2018

This branch has conflicts that must be resolved

Could anyone fix this conflicts files to have pulled this fix ? pushing node_modules is not recommended =/
Thanks!.

@stennie stennie assigned stennie and unassigned gwyneplaine Jun 18, 2018
@stennie stennie added this to the 4.0.0 milestone Jun 18, 2018
@stennie
Copy link
Contributor

stennie commented Jun 18, 2018

Thanks for flagging @dev-dem. It looks like the code fix was already merged to master in #4586, but tests were not updated as per this PR. Changes will be included in the next Keystone release.

Regards,
Stennie

@dev-dem
Copy link

dev-dem commented Jun 18, 2018

@stennie, thank you for the fast answer !

Best!
Dem.-

@stennie
Copy link
Contributor

stennie commented Jun 22, 2018

FYI, a code fix for this issue is included in Keystone 4.0.0-rc.0 which is now available via npm. Release notes: https://github.com/keystonejs/keystone/releases/tag/v4.0.0-rc.0.

This PR is still open pending review of the changes for tests.

Regards,
Stennie

@stennie stennie changed the title Fix query params sagas, custom admin path Update tests for custom admin path Jun 29, 2018
@stennie stennie removed this from the 4.0.0 milestone Jun 29, 2018
@stennie stennie added this to the v4.0.0-rc.2 milestone Jul 7, 2018
@stennie stennie removed this from the v4.0.0-rc.2 milestone Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants