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

Allow all pages in API again #2258

Merged
merged 3 commits into from
Mar 10, 2022

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Mar 10, 2022

What is this pull request for?

This reverts the changes made in 59467e8 and 5c3f912

We need to be able to list all pages from the admin API, because admin users need to be able to link pages from other sites and languages. Since we do not have a way right now to restrict pages, languages and sites to certain admin users we need to return all.

References #2207

Notable changes

Change the admin page select UI so that it shows the site/domain and language a page is on, so that admin users can distinguish pages visually.

Screenshots

Alchemy CMS - Contact 2022-03-10 11-21-14

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

This reverts the changes made in 59467e8
and 5c3f912

We need to be able to list all pages from the admin API, because admin users need to be able to link pages from other sites and languages. Since we do not have a way right now to restrict pages, languages and sites to certain admin users we need to return all.

Pending is a change to the page select UI that shows the site/domain and language a page is, so that admin users can distinguish pages visually.
@tvdeyen tvdeyen requested a review from a team March 10, 2022 11:00
@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 10, 2022

@dbwinger does this look good to you?

tvdeyen added 2 commits March 10, 2022 14:25
We want to display the site in the page select.
We want admins to be able to distinguish the page by site and language.
@tvdeyen tvdeyen force-pushed the allow-all-pages-in-api-again branch from 294ba32 to a544ad1 Compare March 10, 2022 13:25
@dbwinger
Copy link
Contributor

@dbwinger does this look good to you?

Good point and yes this looks good to me. Would you want to include something similar for the Node select UI too? I monkeypatched some things to do something like that for the project I worked on but yours looks much better.

@tvdeyen
Copy link
Member Author

tvdeyen commented Mar 10, 2022

@dbwinger does this look good to you?

Good point and yes this looks good to me. Would you want to include something similar for the Node select UI too? I monkeypatched some things to do something like that for the project I worked on but yours looks much better.

@dbwinger can you elaborate what the use case for the node select is? I can barely image a menu node to be referenced on another language or site?

Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@tvdeyen tvdeyen merged commit 63eec69 into AlchemyCMS:main Mar 10, 2022
@tvdeyen tvdeyen deleted the allow-all-pages-in-api-again branch March 10, 2022 16:25
tvdeyen added a commit that referenced this pull request Mar 10, 2022
Backport pull request #2258 from tvdeyen/allow-all-pages-in-api-again
@dbwinger
Copy link
Contributor

@dbwinger can you elaborate what the use case for the node select is? I can barely image a menu node to be referenced on another language or site?

Actually you're right. I don't need to reference a node on another language/site but currently the node selector is searching ALL nodes (no matter the language/site) and doesn't show the language/site so it's not discernable. So either confining results to the current site/language or displaying the site/language would suffice.

For what it's worth, here's my use case. I built a "mega-menu" global page, with an element corresponding to the mega-menu that should show for each menu item. I associate the element with a Node and use that to show it in my navbar. See it on the site at . Screenshot of the editor for that page with the Node selector (with my addition of site/language) outlined in red:
image

dbwinger referenced this pull request Mar 14, 2023
We were missing a third argument in order to be able to use this
permission with the `.accessible_by` scope.
dbwinger referenced this pull request Mar 20, 2023
We were missing a third argument in order to be able to use this
permission with the `.accessible_by` scope.
tvdeyen added a commit to tvdeyen/alchemy_cms that referenced this pull request Jun 30, 2023
… Pages API requests for page select on link overlay"

We need to revert the change made in AlchemyCMS#2439. We return all pages from the API on purpose. As already discussed in AlchemyCMS#2258

The actual issue was introduced in c98605a where we started to scope the pages to the language again. This change needs to be reverted as well.

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

Successfully merging this pull request may close these issues.

3 participants