Skip to content

fix: Unable to create new role if beforeSave hook exists #8474

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
Merged
merged 43 commits into from
Mar 22, 2023
Merged

fix: Unable to create new role if beforeSave hook exists #8474

merged 43 commits into from
Mar 22, 2023

Conversation

yog27ray
Copy link
Contributor

@yog27ray yog27ray commented Mar 10, 2023 •
edited
Loading

Unable to save role when before save hook is present on role.

Closes: #8473

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK

@parse-github-assistant
Copy link

parse-github-assistant bot commented Mar 10, 2023 •
edited
Loading

Thanks for opening this pull request!

@mtrezza mtrezza linked an issue Mar 10, 2023 that may be closed by this pull request
Error "Tried to create an ACL with an invalid permission type." #8473
Open
4 tasks
@mtrezza mtrezza changed the title issue: Unable to create new role when beforeSave hook is present for … fix: Unable to create new role if beforeSave hook exists Mar 10, 2023
@mtrezza mtrezza changed the base branch from release-5.x.x to alpha March 10, 2023 14:05
@mtrezza mtrezza changed the base branch from alpha to release-5.x.x March 10, 2023 14:05
@mtrezza
Copy link
Member

mtrezza commented Mar 10, 2023

This PR has been opened against the LTS branch. Could you change the base branch to alpha to see whether the issue also exists for the latest alpha release of Parse Server?

@yog27ray
Copy link
Contributor Author

@mtrezza created PR on alpha #8476

@mtrezza
Copy link
Member

mtrezza commented Mar 10, 2023

Thanks, let's see what the CI says there...

@yog27ray
Copy link
Contributor Author

@mtrezza PR on alpha branch desired test cases is passing. The failing ones look like flaky test cases. The issue is on the LTS branch.

@mtrezza
Copy link
Member

mtrezza commented Mar 10, 2023 •
edited
Loading

Yes, so this issue is a 5.x issue, good to know. See #8473 (comment).

@mtrezza mtrezza mentioned this pull request Mar 10, 2023
Open
4 tasks
@codecov
Copy link

codecov bot commented Mar 11, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 ⚠️

Comparison is base (0ec9239) 94.12% compared to head (0cff0a7) 94.10%.

❗ Current head 0cff0a7 differs from pull request most recent head a15e63f. Consider uploading reports for the commit a15e63f to get more accurate results

Additional details and impacted files 
@@                Coverage Diff                @@
##           release-5.x.x    #8474      +/-   ##
=================================================
- Coverage          94.12%   94.10%   -0.02%     
=================================================
  Files                183      183              
  Lines              13776    13776              
=================================================
- Hits               12966    12964       -2     
- Misses               810      812       +2
Impacted Files Coverage Δ
src/RestWrite.js 94.51% <100.00%> (-0.30%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yog27ray
Copy link
Contributor Author

@mtrezza I updated the code to fix the issue. But the random test flows are breaking.

@mtrezza
Copy link
Member

mtrezza commented Mar 11, 2023

It seems to be just a flaky test. I've restarted the test, it should pass.

@mtrezza
Copy link
Member

mtrezza commented Mar 14, 2023

Not sure if you can restart tests with your permissions. I can do that, so I'll just restart until they all pass.

@yog27ray
Copy link
Contributor Author

@mtrezza thanks. Can this PR be reviewed now.

@mtrezza
Copy link
Member

mtrezza commented Mar 16, 2023

All the test pass, so that's good; let's just wait for another review and merge.

@mtrezza mtrezza requested a review from a team March 16, 2023 22:24
@yog27ray
Copy link
Contributor Author

@mtrezza by when this PR can be merged and released?

mtrezza
mtrezza approved these changes Mar 22, 2023
View reviewed changes
Copy link
Member

@mtrezza mtrezza 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!

@mtrezza mtrezza merged commit 4f0f0ec into parse-community:release-5.x.x Mar 22, 2023
parseplatformorg pushed a commit that referenced this pull request Mar 22, 2023
@semantic-release-bot
chore(release): 5.4.3 [skip ci]
e9ae435 
## [5.4.3](5.4.2...5.4.3) (2023-03-22)

### Bug Fixes

* Unable to create new role if `beforeSave` hook exists ([#8474](#8474)) ([4f0f0ec](4f0f0ec))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.4.3

@parseplatformorg parseplatformorg added the state:released-5.x.x Released as LTS version label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dblythy dblythy dblythy approved these changes

@mtrezza mtrezza mtrezza approved these changes

Assignees
No one assigned
Labels
state:released-5.x.x Released as LTS version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error "Tried to create an ACL with an invalid permission type."
4 participants
@yog27ray @mtrezza @parseplatformorg @dblythy