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.

Sign up for GitHub

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

Jump to bottom

Passes the user to the restricted function of the block settings #6271

Merged
merged 8 commits into from
Sep 6, 2024
Merged

Passes the user to the restricted function of the block settings #6271

merged 8 commits into from
Sep 6, 2024

Conversation

wesleybl
Copy link
Member

The restricted function is called inside a useMemo. So we can't call useSelector on it. To work around this, we get the user before useMemo and pass it to the restricted function.

fixes #6264

@wesleybl wesleybl requested a review from tiberiuichim August 26, 2024 19:36
@netlify Netlify
Copy link

netlify bot commented Aug 26, 2024 •
edited
Loading

✅ Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit e96e6b5
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/66da0f399aed390008590d45

@wesleybl wesleybl requested a review from davisagli August 26, 2024 19:36
davisagli
davisagli reviewed Aug 28, 2024
View reviewed changes
Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

Looks good. Could you please also mention user in the docs? https://github.com/plone/volto/blob/main/docs/source/development/how-to-restrict-blocks.md

@wesleybl
Passes the user to the restricted function of the block settings
29239b9 
The restricted function is called inside a useMemo. So we can't call
useSelector on it. To work around this, we get the user before useMemo
and pass it to the restricted function.
@wesleybl
Fix news
34364ac
@wesleybl
Document user parameter in restricted function
caf5463
@wesleybl
Copy link
Member Author

Looks good. Could you please also mention user in the docs?

Done

stevepiercy
stevepiercy requested changes Aug 29, 2024
View reviewed changes
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Your change is good. I corrected awkward phrasing of the preceding sentences to improve flow.

docs/source/development/how-to-restrict-blocks.md Outdated Show resolved Hide resolved
docs/source/development/how-to-restrict-blocks.md Outdated Show resolved Hide resolved
@stevepiercy
Apply suggestions from code review
f027424 
Improve flow by removing awkward phrasing.
stevepiercy
stevepiercy approved these changes Aug 29, 2024
View reviewed changes
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Changes applied. LGTM. Please merge when ready.

@wesleybl wesleybl requested a review from davisagli September 5, 2024 19:57
@davisagli davisagli merged commit 3a3b56d into main Sep 6, 2024
71 checks passed
@davisagli davisagli deleted the user_restricted branch September 6, 2024 17:22
sneridagh added a commit that referenced this pull request Sep 14, 2024
@sneridagh
Merge branch 'main' into vitesupport-ssr
640909a 
* main: (97 commits)
  Release 18.0.0-alpha.43
  Release @plone/slate 18.0.0-alpha.18
  Release @plone/client 1.0.0-alpha.17
  Added a notice in the upgrade guide for CookiePlone as the recommended way for generating projects and add-on boilerplate (#6286)
  Fix typo in Changelog for #6285 (#6287)
  Update to Volto 6.0.13 (#6285)
  Fix order component (#6284)
  Remove dangling `.only` in Cypress test (#6282)
  Empty GridBlocks will not be rendered in 'View' (#6280)
  Refactor Content upload modal  (#5047)
  Passes the `user` to the `restricted` function of the block settings (#6271)
  Refactored SelectMetadataField.jsx to functional component (#5570)
  Remove "Generate tabs for items other than folders" field from navigation control panel (#6278)
  Refactor DatetimeWidget (#6213)
  Added make i18n to volto (#6274)
  Place the actions-storybook in correct folder (#6275)
  Refactor ReferenceWidget (#5093)
  Fix error in SortOn component when there is no sort selected. (#6273)
  When user changes location, set the userSession.token value based on cookie (#6079)
  Trivial MyST syntax enhancement
  ...
@wesleybl
Copy link
Member Author

It was missing to do the dispatch in getUser. It may be that the user is not in the store yet. I'll see if I can make a PR.

@avoinea
Copy link
Member

avoinea commented Sep 25, 2024

@wesleybl @sneridagh @davisagli Do you think we can have this backported into Volto 17.x.x?

@wesleybl
Copy link
Member Author

@avoinea we have to finish #6293. I don't see a problem with having that in Volto 17.

@davisagli
Copy link
Member

@wesleybl @avoinea I think it is backwards compatible and could be backported. If someone prepares the PR I can review it.

@sneridagh sneridagh mentioned this pull request Oct 2, 2024
Merged
@avoinea
Copy link
Member

avoinea commented Oct 4, 2024

@wesleybl As #6293 was merged, maybe we can reiterate now the back-port of this to Volto 17.x 🤔

@wesleybl wesleybl mentioned this pull request Oct 4, 2024
Merged
@wesleybl
Copy link
Member Author

wesleybl commented Oct 4, 2024

As #6293 was merged, maybe we can reiterate now the back-port of this to Volto 17.x 🤔

Done in #6375

@stevepiercy stevepiercy mentioned this pull request Oct 5, 2024
Merged
sneridagh
sneridagh reviewed Oct 17, 2024
View reviewed changes
docs/source/development/how-to-restrict-blocks.md
@@ -22,12 +22,16 @@ The function has this signature:
block: BlockConfigBase;
navRoot: Content;
contentType: string;
user: Object
Copy link
Member

Choose a reason for hiding this comment

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

@wesleybl @davisagli The update in the docs are great but you forgot to update @plone/types :)

volto/packages/types/src/config/Blocks.d.ts

Lines 76 to 83 in 9ffe034

restricted:
| ((args: {
properties: Content;
block: BlockConfigBase; // TODO: This has to be extendable
navRoot: Content;
contentType: string;
}) => boolean)
| boolean;

Copy link
Member

Choose a reason for hiding this comment

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

I will take care:

  restricted:
    | ((args: {
        properties: Content;
        block: BlockConfigBase; // TODO: This has to be extendable
        navRoot: Content;
        contentType: string;
        user: User;
      }) => boolean)
    | boolean;

Copy link
Member

Choose a reason for hiding this comment

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

@sneridagh Er, I haven't really bothered to learn much about typescript yet. I kind of assumed that some linter would yell at me if the types need to be updated. Is that not right? If not, how do I know when there are type declarations that need to be updated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sneridagh sneridagh sneridagh left review comments

@davisagli davisagli davisagli approved these changes

@stevepiercy stevepiercy stevepiercy approved these changes

@tiberiuichim tiberiuichim Awaiting requested review from tiberiuichim

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot use useSelector in the "restricted" function of a Block
5 participants
@wesleybl @avoinea @davisagli @stevepiercy @sneridagh