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

fix(api-headless-cms-bulk-actions): empty trash bin processing entries in series #4351

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

leopuleo
Copy link
Contributor

@leopuleo leopuleo commented Oct 25, 2024

Changes

This PR addresses a bug that arose when emptying the trash bin for instances with many tenants.

Previously, the deletion process operated in parallel. Each tenant, locale, or model triggered a subtask to delete all eligible entries in the specific model for removal. This approach caused errors when dealing with instances that had multiple tenants.

With this update, we consolidate the deletion into a single task that iterates through all tenants, locales, and models, deleting the eligible entries in series.

How Has This Been Tested?

Manually

# Conflicts:
#	packages/api-headless-cms-bulk-actions/src/tasks/createEmptyTrashBinsTask.ts
#	packages/api-headless-cms-bulk-actions/src/types.ts
@leopuleo leopuleo added the cms label Oct 25, 2024
@leopuleo leopuleo added this to the 5.41.2 milestone Oct 25, 2024
@leopuleo leopuleo self-assigned this Oct 25, 2024
@leopuleo leopuleo changed the title fix(api-headless-cms-bulk-actions): empty trash bin with synchronous process fix(api-headless-cms-bulk-actions): empty trash bin processing entries in series Oct 25, 2024
@brunozoric brunozoric modified the milestones: 5.41.2, 5.41.1 Oct 25, 2024
@@ -10,7 +10,10 @@ export interface CreateBulkActionGraphQL {

export const createBulkActionGraphQL = (config: CreateBulkActionGraphQL) => {
return new ContextPlugin<HcmsBulkActionsContext>(async context => {
if (!(await isHeadlessCmsReady(context))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isHeadlessCmsReady() already has checks for tenant and locale.

const tenant = context.tenancy.getCurrentTenant();
const locale = context.i18n.getContentLocale();

if (!tenant || !locale || !(await isHeadlessCmsReady(context))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in createBulkActionGraphQL file.

return;
}

const plugin = new CmsGraphQLSchemaPlugin<Context>({
typeDefs: createTypeDefs(models as NonEmptyArray<CmsModel>),
resolvers: createResolvers(models as NonEmptyArray<CmsModel>)
resolvers: createResolvers(models as NonEmptyArray<CmsModel>),
isApplicable: context =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont understand this check. You get tenant on line 10 and then you get it here again, with same method. Isn't it the same?

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

Successfully merging this pull request may close these issues.

2 participants