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-7418: ContentName search criterion added #2364

Merged
merged 7 commits into from
Jun 14, 2024
Merged

Conversation

julitafalcondusza
Copy link
Contributor

@julitafalcondusza julitafalcondusza commented Apr 19, 2024

Question Answer
JIRA Ticket (ibexa/core#312)
Versions master

ContentName search criterion added in dev-doc.

Checklist

  • Text renders correctly
  • Text has been checked with vale
  • Description metadata is up to date
  • Redirects cover removed/moved pages
  • Code samples are working
  • PHP code samples have been fixed with PHP CS fixer

Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

I think the example is wrong, where is it coming from?

According to the constructor only string value is accepted - meaning that multiple values (arrays) are not valid arguments (see: https://github.com/ibexa/core/blob/main/src/contracts/Repository/Values/Content/Query/Criterion/ContentName.php#L16)

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.

I think the example is wrong, where is it coming from?

According to the constructor only string value is accepted - meaning that multiple values (arrays) are not valid arguments (see: https://github.com/ibexa/core/blob/main/src/contracts/Repository/Values/Content/Query/Criterion/ContentName.php#L16)

I confirm what Marek wrote.

Why do we have such inconsistency? In description int(s) representing the Content name. Next in PHP example array of strings ['laptop', 'tablet']. Finally, REST examples with a single string value.
Examples from original PR (ibexa/core#312) are clear. String value supporting wildcards.

Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

The example is still not correct.

Your example:

$query->query = new Criterion\ContentName(['laptop', 'tablet']);

passes an array with two values (laptop, tablet)

According to the code only a single value is accepted (https://github.com/ibexa/core/blob/main/src/contracts/Repository/Values/Content/Query/Criterion/ContentName.php#L16) (string $value means that arrays are not allowed).

Please take a look at the examples in ibexa/core#312 - single arguments are given.

new ContentName('phrase'); 

new ContentName('phra*'); 

new ContentName('*phrase*'); 

new ContentName('phr*se*'); 

are all valid

Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

Looks good now, thank you!

docs/search/criteria_reference/contentname_criterion.md Outdated Show resolved Hide resolved
@julitafalcondusza julitafalcondusza merged commit 8d963d4 into master Jun 14, 2024
4 checks passed
@julitafalcondusza julitafalcondusza deleted the IBX-7418 branch June 14, 2024 08:09
mnocon pushed a commit that referenced this pull request Jul 9, 2024
* ContentName search criterion added

* mkdocs updated

* Fixes after review

* Fixes

* Fix

* typo

* Fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants