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-5630: Added possibility to extend URL schema resolving #242

Merged
merged 37 commits into from
Jul 18, 2023

Conversation

kisztof
Copy link
Contributor

@kisztof kisztof commented Jun 19, 2023

Question Answer
JIRA issue IBX-5630
Type feature
Target Ibexa version v4.5
BC breaks dunno

This PR adds an extension point for resolving URL schema.

New event added ResolveUrlAliasSchemaEvent

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

@webhdx
Copy link
Contributor

webhdx commented Jun 20, 2023

It would be preferred to fix all CI issues before someone takes a look at it. Also short summary of changes is always helpful to make the review process more effective.

@kisztof kisztof changed the title IBX-5630 IBX-5630 Adds possibitlity to extend url schema resolving Jun 20, 2023
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

Overall seems to be good direction. Parts of the system need immediate improvements to avoid increasing technical debt which is already big for NameSchemaService.
Please also make sure that there's integration test coverage for name and url alias generation from schema (some test that checks that in ./tests/integration/Core). If it is and passes - that's good - no BC break. If there's no test like that, it needs to be added.

src/bundle/Core/ApiLoader/RepositoryFactory.php Outdated Show resolved Hide resolved
src/lib/Repository/Helper/SchemaIdentifierExtractor.php Outdated Show resolved Hide resolved
src/lib/Repository/Helper/SchemaIdentifierExtractor.php Outdated Show resolved Hide resolved
src/lib/Repository/Helper/NameSchemaService.php Outdated Show resolved Hide resolved
src/bundle/Core/Resources/config/papi.yml Outdated Show resolved Hide resolved
src/bundle/Core/Resources/config/events.yml Outdated Show resolved Hide resolved
@alongosz alongosz requested a review from a team June 26, 2023 10:25
@alongosz alongosz changed the title IBX-5630 Adds possibitlity to extend url schema resolving IBX-5630 Added possibility to extend URL schema resolving Jun 26, 2023
@alongosz alongosz changed the title IBX-5630 Added possibility to extend URL schema resolving IBX-5630: Added possibility to extend URL schema resolving Jun 26, 2023
@tomaszszopinski tomaszszopinski self-assigned this Jun 27, 2023
@webhdx webhdx marked this pull request as ready for review June 28, 2023 07:11
@kisztof kisztof force-pushed the feature/IBX-5630 branch 3 times, most recently from 0938085 to f597591 Compare June 30, 2023 11:14
@alongosz alongosz force-pushed the feature/IBX-5630 branch 2 times, most recently from 838de1e to 6128c3e Compare July 5, 2023 00:12
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

src/lib/Repository/ContentService.php Show resolved Hide resolved
src/lib/Repository/NameSchema/NameSchemaService.php Outdated Show resolved Hide resolved
src/lib/Repository/NameSchema/NameSchemaService.php Outdated Show resolved Hide resolved
src/lib/Repository/NameSchema/NameSchemaService.php Outdated Show resolved Hide resolved
src/lib/Repository/Repository.php Show resolved Hide resolved
src/lib/Repository/TrashService.php Show resolved Hide resolved
src/lib/Repository/URLAliasService.php Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jul 18, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@alongosz alongosz merged commit 2070020 into main Jul 18, 2023
22 checks passed
@alongosz alongosz deleted the feature/IBX-5630 branch July 18, 2023 12:33
@alongosz
Copy link
Member

Congrats @kisztof 🎉

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

Successfully merging this pull request may close these issues.

10 participants