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

IBX-7959: Create content permission regression with section limitation #1213

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

yvh
Copy link
Contributor

@yvh yvh commented Mar 14, 2024

Fix issue IBX-7959

Question Answer
JIRA issue IBX-7959
Type bug
Target version v4.6
BC breaks no
Doc needed no

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (master for features, the oldest supported for bugs).
  • Asked for a review (ping @ibexa/engineering).

@ciastektk ciastektk requested a review from a team March 15, 2024 07:35
@konradoboza konradoboza requested a review from a team March 15, 2024 07:38
Copy link
Contributor

@webhdx webhdx left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution :)

@webhdx webhdx added Bug Something isn't working Ready for QA labels Mar 15, 2024
@barbaragr barbaragr self-assigned this Mar 15, 2024
Copy link
Contributor

@ViniTou ViniTou left a comment

Choose a reason for hiding this comment

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

Can we check if failing behat test is not related to change?
@barbaragr

@barbaragr
Copy link

Hi, behat tests are failing because of missing Section/View permission.
For editor user created just like it says here https://issues.ibexa.co/browse/IBX-7959 and here https://issues.ibexa.co/browse/IBX-7961 Create button is disabled, but applying changes from this PR results with 403 error: section/view permissions are missing for editor - user cannot enter Content structure, UDW is full of loading spinners. If I add section/view to the editor policies, Content structure works fine, but in UDW there are still some issues with access.

@glye
Copy link
Contributor

glye commented Mar 15, 2024

I verified that this fix, after the last update, also fixes https://issues.ibexa.co/browse/IBX-7961
so I'll close that as a duplicate.

@ciastektk
Copy link
Contributor

ciastektk commented Mar 18, 2024

Hi! @yvh, could you please add these two changes to make your fix complete and allow us to proceed PR?

  1. Add branch from PR IBX-7959: Added ContentInfo::getSectionId strict getter core#348 as a dev dependency of ibexa/core and in your fix use getSectionId method. We're moving away from using magic in codebase.
  2. As @barbaragr mentioned, same error also occurs in UDW. In https://github.com/ibexa/admin-ui/blob/main/src/bundle/Controller/Permission/LanguageLimitationController.php#L47 we should also use $contentInfo->getSectionId();

Thank you in advance.

Copy link
Contributor

@ciastektk ciastektk left a comment

Choose a reason for hiding this comment

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

As described here: #1213 (comment)

@patrickallaert
Copy link

Hey @ciastektk,

Hi! @yvh, could you please add these two changes to make your fix complete and allow us to proceed PR?

  1. Add branch from PR Added ContentInfo::getSectionId core#348 as a dev dependency of ibexa/core and in your fix use getSectionId method. We're moving away from using magic in codebase.
  2. As @barbaragr mentioned, same error also occurs in UDW. In https://github.com/ibexa/admin-ui/blob/main/src/bundle/Controller/Permission/LanguageLimitationController.php#L47 we should also use $contentInfo->getSectionId();

I would disagree with this for the reason mentioned in ibexa/core#348 (comment). Semantic versioning should not allow this.

Copy link

sonarcloud bot commented Mar 19, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@ciastektk ciastektk merged commit 0900a9a into ibexa:4.6 Mar 19, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working QA approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.