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-3957: Made NOP URL aliases not reusable and original #350

Merged
merged 3 commits into from
May 17, 2024

Conversation

barw4
Copy link
Contributor

@barw4 barw4 commented Mar 19, 2024

Question Answer
JIRA issue IBX-3957
Type bug
Target Ibexa version v5.0
BC breaks yes

Reasoning: because nop aliases are not original and they are reusable creating another alias with the same name will break such an alias, however, imo it should be incremented instead. This will prevent creating exactly the same alias with the same parent and as a result, breaking the old one.


Maintainer updates:

Documentation

Breaking change: if there already exists a custom URL alias having path /a/b where a is a virtual (NOP) entry - not pointing to any content - renaming or creating another content on the same level using a colliding name - a in this case - will result in actually creating /a2 entry, as /a is already taken by /a/b path.

(not sure if wording is good here, might need improvement)

QA

More reasoning behind changes and description what happens was provided here ezsystems/ezplatform-kernel#385 (comment)

We need to test system behavior, find things we missed, edge cases, etc.

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).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ezsystems/engineering-team).

Copy link

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

@alongosz alongosz added the Doc needed The changes require some documentation label Mar 19, 2024
@alongosz alongosz requested a review from a team March 19, 2024 14:06
@webhdx webhdx requested a review from a team March 19, 2024 14:13
@tomaszszopinski tomaszszopinski self-assigned this Apr 29, 2024
@barw4 barw4 force-pushed the ibx-3957-nop-aliases-not-reusable branch from 5337dfd to d1663c4 Compare April 30, 2024 10:31
Copy link

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on IbexaDXP 5.0 commerce.

@Steveb-p
Copy link
Contributor

It should not be possible to rebase this onto the current main branch and have the tests pass.

@tomaszszopinski
Copy link

Did you mean "It should now be possible" ? :)

@barw4 barw4 force-pushed the ibx-3957-nop-aliases-not-reusable branch from d1663c4 to 51dfca1 Compare May 10, 2024 13:33
@Steveb-p
Copy link
Contributor

Did you mean "It should now be possible" ? :)

Yeah... it's a typo... 😊

@barw4
Copy link
Contributor Author

barw4 commented May 13, 2024

@Steveb-p CI fails due to PHPStan issues, this might be caused by the new 1.11.0 release (https://github.com/phpstan/phpstan/releases/tag/1.11.0). CI from main is still on older 1.10.x version.

@barw4 barw4 force-pushed the ibx-3957-nop-aliases-not-reusable branch from 45a4e65 to a213b64 Compare May 17, 2024 13:56
Copy link

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

@alongosz alongosz merged commit 2964324 into main May 17, 2024
13 checks passed
@alongosz alongosz deleted the ibx-3957-nop-aliases-not-reusable branch May 17, 2024 14:36
@julitafalcondusza julitafalcondusza removed the Doc needed The changes require some documentation label Jun 19, 2024
barw4 added a commit that referenced this pull request Oct 17, 2024
For more details see https://issues.ibexa.co/browse/IBX-3957 and #350

Breaking change: if there exists a custom URL alias having a path "/a/b" where "a" is a virtual (NOP) entry - not pointing to any content - renaming or creating another content on the same level using a colliding name - "a" in this case - will result in actually creating "/a2" entry, as "/a" is already taken by "/a/b" path.
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.

8 participants