Add ability to use nested fields from relations

[kibblerz]

What does it do?

I adjusted how the paths are parsed to allow for the use of relation fields. So with this PR, you can use a related contenttypes fields in the path.

This PR only supports 1 level of depth when using relations. If the relation returns an array, the first result will be used. Would like to improve this in the future

Why is it needed?

Currently, relational attributes aren't able to be used.

How to test it?

If the content type is called company, with relations to employees:
/company/[employee[name]]/[post]

Related issue(s)/PR(s)

Let us know if this is related to any issue/pull request

[boazpoolman]

Thanks @kibblerz,
I think this is a great feature.

Though it seems as if you have some kind of linting that applies on save that does not match up with the linting of this repo.

Could you check the issues that came out of the pipeline?
https://github.com/boazpoolman/strapi-plugin-sitemap/runs/5668992189?check_suite_focus=true

[kibblerz]

Sure thing!

[kibblerz]

Should be good to go now!

[kibblerz]

Thanks for approving!
When do you think this could be published on npm?

[boazpoolman]

I still have to do a manual review of the new feature. Until then; you can allready use this PR in your project like this;

yarn add boazpoolman/strapi-plugin-sitemap#pull/75/head
npm install boazpoolman/strapi-plugin-sitemap#pull/75/head

Tests are failing

[boazpoolman]

Hey @kibblerz,

I've thouroughly reviewed your PR. Overall it looks great! Thanks for all the work.
I still have some questions & comment which I have mostly included in the code review.

There was still one question I had;
Did you also test this for a multilingual site?

[kibblerz]

I applied all of your recommended edits. I don't have a bilingual site to test on, but I don't believe my code modified the localization logic.

[codecov-commenter]

Codecov Report

Merging #75 (ecedd29) into master (f76085e) will increase coverage by 9.72%.
The diff coverage is 96.87%.

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
+ Coverage   77.77%   87.50%   +9.72%     
==========================================
  Files           1        2       +1     
  Lines          36       72      +36     
  Branches        9       29      +20     
==========================================
+ Hits           28       63      +35     
- Misses          6        9       +3     
+ Partials        2        0       -2     

Flag	Coverage Δ
unit	87.50% <96.87%> (+9.72%)	⬆️

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

Impacted Files	Coverage Δ
server/services/pattern.js	98.30% <96.87%> (+20.52%)	⬆️
server/utils/index.js	38.46% <0.00%> (ø)

---

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 f76085e...ecedd29. Read the comment docs.

[boazpoolman]

@kibblerz Thanks for the changes!

I've tested it and it works pretty much perfect.
Only some issues with a multilingual Strapi install.

For some reason the localizations attribute on a content type looks like this:

localizations: {
  writable: true,
  private: false,
  configurable: false,
  visible: false,
  type: 'relation',
  relation: 'oneToMany',
  target: 'api::page.page'
},

So the target is not actually the query string of the locale content type, but the query string of the current content type we're querying. So this is the oposite of how a "normal" relation works. We would expect target to be the query string of a locale content type.

Also, when we're creating a pattern for a content type with localizations enabled we see duplicate field options to add to the pattern. So duplicate entries are returned by the getAllowedFields service. See screenshot:

When that's finished I think we need to write some tests for the new pattern logic, but all in all this is a great new feature. Thanks for the work you have allready done on this!

Happy easter 🐣

[boazpoolman]

I've updated your PR to address the issues I mentioned above.
The change removes the possibility to add fields from the localizations relation in the URL pattern. I think that should be fine as the locale is really just there for internal use of the @strapi/i18n-plugin package.

Also I have written some tests for the relational pattern logic. Though whilst doing that I noticed there was one case I was a bit confused with when testing. Which was this part: https://github.com/boazpoolman/strapi-plugin-sitemap/pull/75/files#diff-f306920aa698f85f84aa8ebec6081fda2c285173603fd4dd8ef7e692566699ebR82

A relation field can be an array. So I guess that is for when there can be multiple relations. Like a one-to-many relation type. Though then we just use the first relation?
@kibblerz Could you elaborate on that.

[kibblerz]

@boazpoolman Yes it is set to use the first relation when the relation is an array. The only other solution i thought of was to create a different sitemap path for each relation. Do you have a better recommendation?

[boazpoolman]

I agree. It makes sense to have the pattern be like /test/[relation[0].field] to query the first relation and /test/[relation[1].field] to query the second.

If we don’t do that I think we should not allow to add relations to the pattern which are of type array. As you can’t accurately select the relation. Which is fine by me.

[boazpoolman]

@kibblerz I've made some changes and created an issue for Array relations #78.

Could you test it on your side to see if everything still works for you?

If so; I think we can merge this :)

[kibblerz]

Yes, everything's still working!