fix(content-releases): relation between actions and documents #20424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…/strapi into v5/releases-relation-with-entries
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Feranchz
force-pushed
the
v5/releases-relation-with-entries
branch
from
3602706
to
27c1823
Compare
|
Size Change: -83 B (0%) Total Size: 2.54 MB
ℹ️ View Unchanged
|
Marc-Roig
reviewed
Jun 5, 2024
There was a problem hiding this comment.
Overall I find this approach might give us troublesomes with synchronization.
I have identified the following scenarios where a release action could become "obsolete"
- if content type is removed
- if locale is removed
- if document is removed
- if document locale is removed
- if document published version is unpublished (?)
- if i18n is disabled on content type
I am unsure if this approach atm would be enough, as some of those might break the release population. Can we discuss it today ?
packages/core/content-releases/server/src/migrations/database/5.0.0-document-id-in-actions.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+109
to
+121
| const actionStatus = | ||
| action.type === 'publish' | ||
| ? getDraftEntryValidStatus( | ||
| { | ||
| contentType: action.contentType, | ||
| documentId: action.entryDocumentId, | ||
| locale: action.locale, | ||
| }, | ||
| { | ||
| strapi, | ||
| } | ||
| ) | ||
| : true; |
There was a problem hiding this comment.
Unpublishing also should trigger validations, as there could be unique fields in the draft that might be broken
| const releaseService = getService('release', { strapi }); | ||
| const { results, pagination } = await releaseService.findActions(releaseId, { | ||
| const releaseActionService = getService('release-action', { strapi }); | ||
| const { results, pagination } = await releaseActionService.findPage(releaseId, { | ||
| ...query, | ||
| }); |
There was a problem hiding this comment.
the query filters should be sanitized, users could be arbirarily populating any kind of field if not
There was a problem hiding this comment.
Yes, some lines before is sanitized, do you think the changes on .sort could be a problem? 🤔
Marc-Roig
reviewed
Jun 5, 2024
Comment on lines
+171
to
+180
| entry: await getEntry( | ||
| { | ||
| contentType: action.contentType, | ||
| documentId: action.entryDocumentId, | ||
| locale: action.locale, | ||
| populate, | ||
| status: action.type === 'publish' ? 'draft' : 'published', | ||
| }, | ||
| { strapi } | ||
| ), |
There was a problem hiding this comment.
what happens if the content type doesnt exist? (it has been deleted)
There was a problem hiding this comment.
That case should be handle by the functions on db lifecycles, currently we delete actions if content type is deleted or D&P is disabled on the Content Type
|
If this is the approach we commit the docs should be updated too, also do we have api tests for releases? |
Marc-Roig
reviewed
Jun 5, 2024
…es-relation-with-entries
Feranchz
force-pushed
the
v5/releases-relation-with-entries
branch
from
077b49d
to
620cbaa
Compare
Feranchz
force-pushed
the
v5/releases-relation-with-entries
branch
from
808d56d
to
bdb9595
Compare
Feranchz
added a commit
that referenced
this pull request
Jun 13, 2024
* chore: migrate bulk publish & unpublish to v5 * chore: change findLocales type to accept arrays * chore: fix lint error * chore: migrate bulkDelete to v5 (#20161) * chore: migrate bulkDelete to v5 * chore: change findLocales type to accept strings array * fix: docs prettier styles * chore: remove console.log * enhancement: migrate countManyDraftRelations to v5 * chore(content-releases): v5 migration * chore(content-releases): remove CMReleasesContainer * fix(content-releases): singleType works with v5 changes and e2e tests enabled * fix(content-releases): apply josh & marc comments * apply comments * fix(content-releases): tests * fix(content-releases): create custom populate object for each content type to handle relations * fix(content-releases): build problem * fix(content-releases): editing lifecycles * fix(content-releases): details view table columns * feat: releases settings (#20354) * feat: releases settings * feat: test nulling default timezone * chore: refactor tests * fix: remove async from describe * chore: OneOf type for response * chore: move OneOf utility to types package --------- Co-authored-by: Fernando Chavez <fernando.chavez@strapi.io> * feat: content releases settings permissions (#20357) * feat: releases settings * feat: test nulling default timezone * chore: refactor tests * fix: remove async from describe * feat: content releases settings permissions * chore: test for unauthorized role --------- Co-authored-by: Fernando Chavez <fernando.chavez@strapi.io> * fix(content-releases): run settings api tests only on ee edition * fix(content-releases): apply mark comments * fix(content-releases): remove releases box when there are no releases related to the entry * fix(content-releases): relation between actions and documents (#20424) * fix(content-releases): refactor relation with entries * fix(content-releases): refactor relation with entries * fix(content-releases): lint & unit tests errors * fix(content-releases): add migration for releases actions coming from v4 * fix(content-releases): apply multiple suggestions * fix(content-releases): new test data for e2e tests * fix(content-releases): fix test data * fix(content-releases): handle edge cases * fix(content-releases): apply marc suggestions * fix(content-releases): add modified status on validation column * fix(content-releases): fix releases menu button * fix(content-releases): use documents middleware instead of db lifecycles * fix(content-releases): invalidate releases tags on some content manager queries * fix(content-releases): using contentType utils and make afterDeleteMany lifecycle async * fix(content-releases): ui fixs * fix(content-releases): removing not needed axios from releases plugin * fix(content-releases): invalidate tags on release service * fix(content-releases): fix dependencies * feat(release-settings): remove navbar link release purchase page in CE (#20498) --------- Co-authored-by: Marc Roig <marc.roig.campos@strapi.io> Co-authored-by: Simone <startae14@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does it do?
This change how Releases Actions are related to entries. Before we used polymorphic relationships between a release action and the entry, this seems to create different problems in v5 with the new documents layer.
Currently, polymorphic relations don't work with documents id and they need to be handled still with entry Id. This creates the following problem: What entry Id should I use based on the release action type? If we are publishing the entry we want the draft entry id, if we are unpublishing we want the published entry id.
Considering that you can change the release action type after adding entries to a release, this creates inconsistency between the different data.
Proposed solution
Stop using polymorphic relations between release actions and entries (at least until they are completely implemented on v5) and save all the needed information in Release Action schema to get the document related (Noticed I want to say document related and not entry related)
This information is
contentTypeUid,localeand entry'sdocumentId. We already save the first two on the Release Action schema, so we are only adding entry's documentId.Known issues
Work to do
with-admindata we are using is not adapted to this new format.