Skip to content

Conversation

@petersg83
Copy link
Contributor

What does it do?

Removes the hard limit on deep populate for an entity.
(It was not possible before but since we populate relations only once, it is possible to remove the limit)

Why is it needed?

In order to populate the admin edit view when there are more than 2 levels of nested components.

How to test it?

  1. Create compo1 with a field "name"
  2. Create compo2 with a field "compo1" and a field "name"
  3. Create compo3 with a field "compo2" and a field "name"
  4. Create a content-type "dog" with a field "compo3"
  5. Create an entry for the content-type "dog" with all the components filled
  6. Save and see that the data is correctly displayed

NB: component creation is possible only by editing the files directly, not through the content-type builder.

Related issue(s)/PR(s)

@petersg83 petersg83 added issue: bug Issue reporting a bug source: core:admin Source is core/admin package labels May 16, 2022
@petersg83 petersg83 added this to the 4.1.12 milestone May 16, 2022
@petersg83
Copy link
Contributor Author

Does it have to be considered as a new feature since it's on canny?
I would say no as it only displays the data for people that bypass the 2 levels logic. Currently the Content-Type Builder doesn't allow to create nested components.

@petersg83 petersg83 added pr: fix This PR is fixing a bug source: core:content-manager Source is core/content-manager package and removed issue: bug Issue reporting a bug source: core:admin Source is core/admin package labels May 16, 2022
@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #13331 (2982fa7) into master (b2980ab) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master   #13331      +/-   ##
==========================================
- Coverage   47.92%   47.91%   -0.01%     
==========================================
  Files         239      239              
  Lines        8814     8813       -1     
  Branches     1984     1982       -2     
==========================================
- Hits         4224     4223       -1     
  Misses       3783     3783              
  Partials      807      807              
Flag Coverage Δ
front ?
unit 47.91% <66.66%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
.../content-manager/server/services/entity-manager.js 31.19% <66.66%> (-0.34%) ⬇️
.../server/services/helpers/build-component-schema.js 90.62% <0.00%> (-0.29%) ⬇️
packages/core/database/lib/migrations/index.js 30.00% <0.00%> (+1.42%) ⬆️

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 b2980ab...2982fa7. Read the comment docs.

alexandrebodin
alexandrebodin previously approved these changes May 23, 2022
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM

@alexandrebodin alexandrebodin dismissed their stale review May 23, 2022 10:16

Missed sth

@alexandrebodin alexandrebodin modified the milestones: 4.1.12, 4.2.0, 4.1.13 May 25, 2022
@derrickmehaffy
Copy link
Member

Does it have to be considered as a new feature since it's on canny? I would say no as it only displays the data for people that bypass the 2 levels logic. Currently the Content-Type Builder doesn't allow to create nested components.

I don't think so, we can consider this whatever we want but the canny card should be updated once this is merged (I'm gonna change it to in progress right now)

@derrickmehaffy
Copy link
Member

@petersg83 petersg83 requested a review from alexandrebodin May 31, 2022 13:55
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM

@alexandrebodin alexandrebodin merged commit c006f74 into master May 31, 2022
@alexandrebodin alexandrebodin deleted the fix/deep-populate-components branch May 31, 2022 13:57
@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/multiple-nested-repeatable-components/665/31

@Convly Convly removed this from the 4.1.13 milestone Jun 15, 2022
@Convly Convly added this to the 4.2.0 milestone Jun 15, 2022
@Warxcell
Copy link

image

@yanniskadiri yanniskadiri mentioned this pull request Jun 24, 2022
@EVSHK
Copy link

EVSHK commented Jun 24, 2022

Hello, i tried the How to test it exemple and it works on Strapi Admin side by editing the files directly.
But when i try to fetchAPI this page the response is not complete. Got only 1 sub-level.

try fetch 1 : const res = await fetchAPI("/testPage", {populate: "*"});
try fetch 2 : const res = await fetchAPI("/testPage", {populate: {content: {populate: "*"}}});

Can you help ? Maybe i missed something on te populate part or ?

Capture d’écran 2022-06-24 à 16 19 22
Capture d’écran 2022-06-24 à 16 18 28
Capture d’écran 2022-06-24 à 16 17 39

@paddotk
Copy link

paddotk commented Oct 12, 2022

Hi,
We're on Strapi v4.4.3 and it doesn't look like multi-level component nesting works yet. I've manually edited the json files to make this work, however whenever I try to add a field (a plain one like 'text' for example), or try changing a collection/instance's name in a nested component in the Strapi interface, it won't save. The error I see in my devtools' network tab is row already is a nested component. You cannot have more than one level of nesting inside your components..

@derrickmehaffy
Copy link
Member

Once you edit the schema manually to go more than two levels you have continue to edit all of those schemas manually as the content-type builder will no longer work for those content-types or components.

This was intended for us to discourage users from nesting components too deeply.

@axufurisamu
Copy link

@derrickmehaffy How about instead of discouraging users from using a very common method in CMS for nested components, and something you all had in the previous version, you actually fix the issue and allow us to nest components!

@derrickmehaffy
Copy link
Member

@derrickmehaffy How about instead of discouraging users from using a very common method in CMS for nested components, and something you all had in the previous version, you actually fix the issue and allow us to nest components!

By doing so it means we have to support it's usage both from a higher level UX but also from the data level and quite simply put, we don't want to because the complex database schema and performance impacts of such are two things that we are not confident in nor is it a methodology we want to support. We unlocked the ability (intentional block) of nested components, by choosing to use them that way you do so at your own risk without us saying if they break we will fix them. There is nothing basic about this feature, they are complex polymorphic relations.

Our plan is to have the ability to create new relational entities in the same parent view in the future, components are not the end-all be-all of a CMS and we are a heavily relational CMS, not an object storage one.

I'm locking this issue as there is nothing constructive being added to this. We have taken a decision on the topic for now, that decision was made by our product team @Aurelsicoko and @yanniskadiri . If you want to create a feature request for this on https://feedback.strapi.io and collect votes it can be re-evaluated later but I would please ask that you watch your tone to ensure it meets our community code of conduct.

@strapi strapi locked and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

pr: fix This PR is fixing a bug source: core:content-manager Source is core/content-manager package

Projects

None yet

Development

Successfully merging this pull request may close these issues.