8000 Add test for Content API filter by locale by iicdii · Pull Request #11961 · strapi/strapi · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Ad 8000 d test for Content API filter by locale #11961

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
Merged
merged 8 commits into from
Dec 21, 2021
Merged

Add test for Content API filter by locale #11961

merged 8 commits into from
Dec 21, 2021

Conversation

iicdii
Copy link
Contributor
@iicdii iicdii commented Dec 21, 2021 •
edited
Loading

Signed-off-by: harimkims harimkims@gmail.com

What does it do?

  • Add e2e tests for filtering by locale (collection-type/single-type)

Why is it needed?

- To enable filter by locale

Following the documentation, you can filter content by _locale parameter but the identifier was locale in code.

It turns out the documentation didn't reflect the change the usage of _locale becomes locale in i18n api.
So I revert changes and just keep the e2e tests.

How to test it?

send GET request with locale

  • ex) http://localhost:1337/api/homepage?locale=ko
  • http://localhost:1337/api/homepage?locale=en
  • http://localhost:1337/api/homepage?locale=all

Related issue(s)/PR(s)

Closes #11953

@iicdii
Fix unable to filter by locale
719e7a6 
Signed-off-by: harimkims <harimkims@gmail.com>
@iicdii
Fix failed e2e test
51abb6e
@codecov
Copy link
codecov bot commented Dec 21, 2021 •
edited
Loading

Codecov Report

Merging #11961 (ac78cea) into master (4497124) will not change coverage.
The diff coverage is n/a.

❗ Current head ac78cea differs from pull request most recent head b2b0f85. Consider uploading reports for the commit b2b0f85 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11961   +/-   ##
=======================================
  Coverage   47.63%   47.63%           
=======================================
  Files         211      211           
  Lines        8200     8200           
  Branches     1856     1856           
=======================================
  Hits         3906     3906           
  Misses       3542     3542           
  Partials      752      752
Flag Coverage Δ
front ?
unit 47.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4497124...b2b0f85. Read the comment docs.

@Convly
Copy link
Member
Convly commented Dec 21, 2021

Hi, thank you for opening this PR.

There is indeed an issue between the code only allowing the locale filter and the documentation talking about a _locale filter.

As we moved from _locale to locale in V4, I don't think we want to support both formats, thus I don't think your change is necessary. Regardless, the tests are very helpful and it could be cool to keep them.

Would you mind updating your PR to reflect this, please? 🙂

@iicdii
Copy link
Contributor Author
iicdii commented Dec 21, 2021

Sure, I will just keep the test and revert others.

@iicdii iicdii changed the title Fix unable to filter by locale Add test for content-api filter by locale Dec 21, 2021
@iicdii iicdii changed the title Add test for content-api filter by locale Add test for Content API filter by locale Dec 21, 2021
@Convly Convly mentioned this pull request Dec 21, 2021
Merged
@Convly Convly added issue: enhancement Issue suggesting an enhancement to an existing feature source: plugin:i18n Source is plugin 8000 /i18n package labels Dec 21, 2021
@Convly Convly added this to the 4.0.1 milestone Dec 21, 2021
Convly
Convly requested changes Dec 21, 2021
View reviewed changes
Copy link
Member
@Convly Convly left a comment

Choose a reason for hiding this comment

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

Just some wording adjustment but otherwise it's 💯

Convly
Convly approved these changes Dec 21, 2021
View reviewed changes
8000
Copy link
Member
@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for the tests to pass, then merging 🔥

@Convly Convly merged commit 93de45c into strapi:master Dec 21, 2021
@iicdii iicdii deleted the fix/unable-to-filter-by-locale branch December 21, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Convly Convly Convly approved these changes

Assignees
No one assigned
Labels
issue: enhancement Issue suggesting an enhancement to an existing feature source: plugin:i18n Source is plugin/i18n package
Projects
None yet
Milestone
4.0.1
Development
3E41

Successfully merging this pull request may close these issues.

Cannot get single type in the selected locale
2 participants
@iicdii @Convly
0