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-6282: Added attributes support for generating name schema #257

Merged
merged 37 commits into from
Sep 19, 2023

Conversation

kisztof
Copy link
Contributor

@kisztof kisztof commented Aug 7, 2023

Question Answer
JIRA issue IBX-6282
Type improvement
Target Ibexa version v4.6
BC breaks no

This PR added significant changes to content naming in the system, introducing custom name schemas using attributes. The name schema pattern used in these events matches the URL Alias schema pattern for consistency.

Added two new events:

  • \Ibexa\Contracts\Core\Event\NameSchema\ResolveNameSchemaEvent and \Ibexa\Contracts\Core\Event\NameSchema\ResolveContentNameSchemaEvent, enabling dynamic name schemas for content.

Refactored names:

  • resolve() => resolveNameSchema()
  • resolveNameSchema() => resolveContentNameSchema()

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that the 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).

@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch 2 times, most recently from 0aa6e59 to 48b6411 Compare August 9, 2023 11:28
@kisztof kisztof requested a review from a team August 9, 2023 21:18
@kisztof kisztof marked this pull request as ready for review August 9, 2023 21:22
@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch from 2651348 to 1cf2892 Compare August 9, 2023 21:37
@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch from 194b24a to 0bc36a4 Compare August 17, 2023 12:56
src/contracts/Event/NameSchema/AbstractSchemaEvent.php Outdated Show resolved Hide resolved
src/contracts/Event/NameSchema/AbstractSchemaEvent.php Outdated Show resolved Hide resolved
src/contracts/Event/NameSchema/AbstractSchemaEvent.php Outdated Show resolved Hide resolved
src/contracts/Event/NameSchema/AbstractSchemaEvent.php Outdated Show resolved Hide resolved
src/lib/Repository/Helper/NameSchemaService.php Outdated Show resolved Hide resolved
src/lib/Repository/Helper/NameSchemaService.php Outdated Show resolved Hide resolved
src/lib/Repository/NameSchema/NameSchemaService.php Outdated Show resolved Hide resolved
Copy link
Contributor

@konradoboza konradoboza left a comment

Choose a reason for hiding this comment

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

Also, this this remark hasn't been incorporated: https://github.com/ibexa/core/pull/257/files#r1304080695.

src/lib/Repository/Helper/NameSchemaService.php Outdated Show resolved Hide resolved
src/lib/Repository/Helper/NameSchemaService.php Outdated Show resolved Hide resolved
@micszo micszo self-assigned this Aug 30, 2023
@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch from 3e0fce6 to 41217c4 Compare August 31, 2023 08:23
Copy link
Contributor

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Publishing product with name pattern <name>-<attribute:battery_type>-<attribute:disk_size> causes error:

Ibexa\ProductCatalog\Event\NameSchemaSubscriber::getProductSpecificationFieldFromEvent(): Return value must be of type ?Ibexa\ProductCatalog\FieldType\ProductSpecification\Value, array returned

@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch 2 times, most recently from 2eb8749 to 12150d1 Compare September 6, 2023 09:36
@micszo
Copy link
Contributor

micszo commented Sep 7, 2023

With this change content name pattern is ignored when different URL alias name pattern is provided:

Screenshot 2023-09-07 at 09 33 03
Screenshot 2023-09-07 at 10 41 31
Screenshot 2023-09-07 at 10 41 46
Screenshot 2023-09-07 at 10 41 55

@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch 3 times, most recently from 5feb230 to d111bde Compare September 11, 2023 08:29
@micszo micszo self-requested a review September 13, 2023 12:00
@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch from d0a0698 to 1ca59c0 Compare September 13, 2023 12:29
@webhdx webhdx requested a review from alongosz September 13, 2023 13:19
@kisztof kisztof force-pushed the feature/IBX-6282-name-schema branch from 5e8370d to 0359960 Compare September 14, 2023 13:18
@sonarqubecloud
Copy link

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 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Using attributes in product name pattern works fine now.
Previous issues have been resolved.

QA Approved on Ibexa Commerce 4.6.x-dev.

@micszo micszo removed their assignment Sep 19, 2023
@alongosz alongosz merged commit 3e143a0 into main Sep 19, 2023
22 checks passed
@alongosz alongosz deleted the feature/IBX-6282-name-schema branch September 19, 2023 10:39
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.

8 participants