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.

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

IBX-2000: Added URL Wildcard Query with Criterion and Sort #301

Merged
merged 13 commits into from
May 25, 2022
Merged

IBX-2000: Added URL Wildcard Query with Criterion and Sort #301

merged 13 commits into from
May 25, 2022

Conversation

mateuszdebinski
Copy link
Contributor

@mateuszdebinski mateuszdebinski commented Apr 21, 2022 •
edited
Loading

Question Answer
JIRA issue IBX-2000
Type improvement
Target Ibexa version v3.3
BC breaks no

Prepare to deploy Query and Criterion to Wildcard URL to add UI search

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).

@mateuszdebinski mateuszdebinski changed the title IBX-2000: Added UWL Wildcard Query with Criterion and Sort IBX-2000: Added URL Wildcard Query with Criterion and Sort Apr 21, 2022
adamwojs
adamwojs reviewed Apr 22, 2022
View reviewed changes
@mateuszdebinski mateuszdebinski mentioned this pull request Apr 22, 2022
Merged
2 tasks
alongosz
alongosz requested changes Apr 29, 2022
View reviewed changes
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.

An integration test coverage for all criteria should be added in the new namespace \Ibexa\Tests\Integration\Core\Repository\URLWildcardService\*
(based on \eZ\Publish\API\Repository\Tests\URLWildcardServiceTest).
Remarks:

@mateuszdebinski mateuszdebinski marked this pull request as ready for review May 18, 2022 08:03
@mateuszdebinski mateuszdebinski requested review from adamwojs, alongosz and a team May 18, 2022 08:03
@mateuszdebinski mateuszdebinski self-assigned this May 18, 2022
@mateuszdebinski mateuszdebinski added the Improvement Changes not fixing or changing behavior label May 18, 2022
alongosz
alongosz requested changes May 18, 2022
View reviewed changes
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.

@mateuszdebinski Sonar analysis seems to be relevant here.
Other remarks:

@mateuszdebinski mateuszdebinski requested a review from alongosz May 18, 2022 16:14
@alongosz alongosz requested review from Steveb-p, mikadamczyk and a team May 19, 2022 13:30
This was referenced May 20, 2022
Closed
Closed
alongosz
alongosz requested changes May 20, 2022
View reviewed changes
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.

LGTM, mostly cosmetic requests:

@mateuszdebinski mateuszdebinski requested a review from alongosz May 20, 2022 10:55
@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 6 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

tomaszszopinski
tomaszszopinski approved these changes May 25, 2022
View reviewed changes
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 3.3 commerce.

@adamwojs adamwojs merged commit dcc6050 into 1.3 May 25, 2022
@adamwojs adamwojs deleted the IBX-2000_add_search_to_url_wildcard_tab branch May 25, 2022 12:39
@adamwojs
Copy link
Member

Could you please merge up changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamwojs adamwojs adamwojs approved these changes

@Steveb-p Steveb-p Steveb-p approved these changes

@alongosz alongosz alongosz approved these changes

@webhdx webhdx webhdx approved these changes

@tomaszszopinski tomaszszopinski tomaszszopinski approved these changes

@mikadamczyk mikadamczyk Awaiting requested review from mikadamczyk

Assignees

@adamwojs adamwojs

@mikadamczyk mikadamczyk

@Steveb-p Steveb-p

@alongosz alongosz

@ciastektk ciastektk

@mateuszdebinski mateuszdebinski

Labels
Improvement Changes not fixing or changing behavior QA approved
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mateuszdebinski @adamwojs @mikadamczyk @Steveb-p @alongosz @webhdx @tomaszszopinski @ciastektk