Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions src/Umbraco.Web.UI.Client/mocks/db/document-publishing.manager.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { UmbMockDocumentModel } from '../data/mock-data-set.types.js';

Check warning on line 1 in src/Umbraco.Web.UI.Client/mocks/db/document-publishing.manager.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/improvement/save-and-publish-take-three)

❌ New issue: String Heavy Function Arguments

In this module, 46.7% of all arguments to its 9 functions are strings. The threshold for string arguments is 39.0%. The functions in this file have a high ratio of strings as arguments. Avoid adding more.
import type { UmbDocumentMockDB } from './document.db.js';
import type {
CreateAndPublishDocumentRequestModel,
PublishDocumentRequestModel,
PublishDocumentWithDescendantsRequestModel,
UnpublishDocumentRequestModel,
UpdateAndPublishDocumentRequestModel,
} from '@umbraco-cms/backoffice/external/backend-api';
import { UmbId } from '@umbraco-cms/backoffice/id';
import type { DocumentVariantResponseModel } from '@umbraco-cms/backoffice/external/backend-api';
Expand Down Expand Up @@ -55,6 +57,34 @@
this.#documentDb.detail.update(id, document);
}

createAndPublish(data: CreateAndPublishDocumentRequestModel) {
const id = this.#documentDb.detail.create(data);
this.#publishCultures(id, data.culturesToPublish);
return id;
}

updateAndPublish(id: string, data: UpdateAndPublishDocumentRequestModel) {
this.#documentDb.detail.update(id, data);
this.#publishCultures(id, data.culturesToPublish);
}

#publishCultures(id: string, culturesToPublish: Array<string>) {
const document: UmbMockDocumentModel = this.#documentDb.detail.read(id);

// Invariant content types publish with an empty cultures array; publish the invariant variant in that case.
const cultures: Array<string | null> = culturesToPublish.length > 0 ? culturesToPublish : [null];

cultures.forEach((culture) => {
const variant = document.variants.find((x) => x.culture === culture);
if (variant) {
variant.state = 'Published' as UmbDocumentVariantState;
variant.updateDate = new Date().toISOString();
}
});

this.#documentDb.detail.update(id, document);
}

publishWithDescendants(id: string, data: PublishDocumentWithDescendantsRequestModel) {
const document: UmbMockDocumentModel = this.#documentDb.detail.read(id);
const documents = this.getDescendants(id, []);
Expand All @@ -64,10 +94,7 @@
for (const culture of data.cultures) {
for (const d of documents) {
const variant = document.variants.find((x) => x.culture === culture);
if (
variant &&
(data.includeUnpublishedDescendants || variant.state !== 'Published')
) {
if (variant && (data.includeUnpublishedDescendants || variant.state !== 'Published')) {
variant.state = 'Published' as UmbDocumentVariantState;
variant.updateDate = new Date().toISOString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import { umbMockManager } from '../../mock-manager.js';
import { umbDocumentMockDb } from '../../db/document.db.js';
import { UMB_SLUG } from './slug.js';
import type {
CreateAndPublishDocumentRequestModel,
CreateDocumentRequestModel,
DefaultReferenceResponseModel,
GetDocumentByIdAvailableSegmentOptionsResponse,
GetDocumentByIdReferencedDescendantsResponse,
PagedIReferenceResponseModel,
UpdateAndPublishDocumentRequestModel,
UpdateDocumentRequestModel,
} from '@umbraco-cms/backoffice/external/backend-api';
import { umbracoPath } from '@umbraco-cms/backoffice/utils';
Expand Down Expand Up @@ -38,6 +40,21 @@ export const detailHandlers = [
});
}),

http.post(umbracoPath(`${UMB_SLUG}/create-and-publish`), async ({ request }) => {
const requestBody = (await request.json()) as CreateAndPublishDocumentRequestModel;
if (!requestBody) return new HttpResponse(null, { status: 400, statusText: 'no body found' });

const id = umbDocumentMockDb.publishing.createAndPublish(requestBody);

return HttpResponse.json(null, {
status: 201,
headers: {
Location: request.url + '/' + id,
'Umb-Generated-Resource': id,
},
});
}),

http.get(umbracoPath(`${UMB_SLUG}/configuration`), () => {
return HttpResponse.json(umbDocumentMockDb.getConfiguration());
}),
Expand Down Expand Up @@ -155,6 +172,19 @@ export const detailHandlers = [
}
}),

http.put(umbracoPath(`${UMB_SLUG}/:id/update-and-publish`), async ({ request, params }) => {
const id = params.id as string;
if (!id) return new HttpResponse(null, { status: 400 });
if (id === 'forbidden') {
// Simulate a forbidden response
return new HttpResponse(null, { status: 403 });
}
const requestBody = (await request.json()) as UpdateAndPublishDocumentRequestModel;
if (!requestBody) return new HttpResponse(null, { status: 400, statusText: 'no body found' });
umbDocumentMockDb.publishing.updateAndPublish(id, requestBody);
return new HttpResponse(null, { status: 200 });
}),

http.put(umbracoPath(`${UMB_SLUG}/:id`), async ({ request, params }) => {
const id = params.id as string;
if (!id) return new HttpResponse(null, { status: 400 });
Expand Down

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { UMB_DOCUMENT_WORKSPACE_CONTEXT } from '../../workspace/context/document-workspace.context-token.js';

Check notice on line 1 in src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/workspace-context/document-publishing.workspace-context.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/improvement/save-and-publish-take-three)

✅ Getting better: Overall Code Complexity

The mean cyclomatic complexity decreases from 6.78 to 6.72, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
import type {
UmbDocumentDetailModel,
UmbDocumentVariantOptionModel,
Expand Down Expand Up @@ -399,33 +399,36 @@
const entityType = this.#documentWorkspaceContext.getEntityType();
if (!entityType) throw new Error('Entity type is missing');

await this.#documentWorkspaceContext.performCreateOrUpdate(variantIds, saveData);
// Capture the current (draft) data before the operation, so variants that are not being published
// but still hold unsaved edits can be kept dirty after the reload below.
const dirtyData = this.#documentWorkspaceContext.getData();

const { error } = await this.#publishingRepository.publish(
unique,
variantIds.map((variantId) => ({ variantId })),
);
// Save and publish in a single server transaction (replaces the separate save + publish calls).
await this.#documentWorkspaceContext.performCreateOrUpdateAndPublish(variantIds, saveData);

if (!error) {
this.#notificationContext?.peek('positive', {
data: {
message: this.#localize.term('speechBubbles_editContentPublishedHeader'),
},
});
this.#notificationContext?.peek('positive', {
data: {
message: this.#localize.term('speechBubbles_editContentPublishedHeader'),
},
});

// Clear stale published data and pending changes state so the
// persistedData observer does not run a comparison against outdated
// data during reload, which would briefly show a false-positive
// "pending changes" state.
this.#clear();
// Clear stale published data and pending changes state so the
// persistedData observer does not run a comparison against outdated
// data during reload, which would briefly show a false-positive
// "pending changes" state.
this.#clear();

// reload the document so all states are updated after the publish operation
await this.#documentWorkspaceContext.reload();
await this.#loadAndProcessLastPublished();
// reload the document so all states are updated after the publish operation
await this.#documentWorkspaceContext.reload();

const event = new UmbRequestReloadStructureForEntityEvent({ unique, entityType });
this.#eventContext?.dispatchEvent(event);
}
// The reload resets the current data state to the full server document. Re-apply the edits of the
// variants that were not published, so they remain dirty (and the Discard-Changes guard fires).
await this.#documentWorkspaceContext.transferPublishedVariantsToCurrent(dirtyData, variantIds);
Comment on lines +417 to +428
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is something about this part that we need to look into.
I did a little bit of investigation on it, and from how things works with Discard Changes, then this seems like the only option. But to me there is a lot of arrows pointing towards how the Dicard Changes mechanisme works.
The reason why it itches a bit, from an architectural standpoint, is that for un-saved variants we shortly change it to be like it is on the server and then revert back to the client-draft. To avoid the Discard Changes, I would rather like the discard changes to understand that switching between Create/Edit should not trigger the check for unsaved changes logic.
I would like to dive further into this at one point, but this is currently my only comment on this PR, so putting it here to leave that thought available for who comes around.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right — and that reconcile-then-revert is the bit that bugs me too, not just you. Presenting an unsaved variant as the server state for a split second and then flipping it back is the kind of thing that works but never sits quite right. Agree the real fix is teaching the guard that Create→Edit isn't a navigate-away, rather than dancing around it out here. Didn't want to drag it into this PR though since it's shared infra (media and the rest go through the same redirect), so I've split it out: #23086. Thanks for digging into it 🙂


await this.#loadAndProcessLastPublished();

const event = new UmbRequestReloadStructureForEntityEvent({ unique, entityType });
this.#eventContext?.dispatchEvent(event);

Check notice on line 431 in src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/workspace-context/document-publishing.workspace-context.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/improvement/save-and-publish-take-three)

✅ Getting better: Complex Method

UmbDocumentPublishingWorkspaceContext.performSaveAndPublish decreases in cyclomatic complexity from 10 to 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

#publishableVariantsFilter = (option: UmbDocumentVariantOptionModel) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,41 @@ import { UmbDocumentServerDataSource } from './document-detail.server.data-sourc
import { UMB_DOCUMENT_DETAIL_STORE_CONTEXT } from './document-detail.store.context-token.js';
import type { UmbControllerHost } from '@umbraco-cms/backoffice/controller-api';
import { UmbDetailRepositoryBase } from '@umbraco-cms/backoffice/repository';
export class UmbDocumentDetailRepository extends UmbDetailRepositoryBase<UmbDocumentDetailModel> {
import type { UmbVariantId } from '@umbraco-cms/backoffice/variant';
export class UmbDocumentDetailRepository extends UmbDetailRepositoryBase<
UmbDocumentDetailModel,
UmbDocumentServerDataSource
> {
constructor(host: UmbControllerHost) {
super(host, UmbDocumentServerDataSource, UMB_DOCUMENT_DETAIL_STORE_CONTEXT);
}

/**
* Creates and publishes a new Document in a single operation
* @param {UmbDocumentDetailModel} model - The Document to create
* @param {Array<UmbVariantId>} variantIds - The variants to publish after creating
* @param {string | null} parentUnique - The unique of the parent to create under
* @returns {*}
* @memberof UmbDocumentDetailRepository
*/
async createAndPublish(
model: UmbDocumentDetailModel,
variantIds: Array<UmbVariantId>,
parentUnique: string | null = null,
Comment thread
iOvergaard marked this conversation as resolved.
Outdated
) {
return this.detailDataSource.createAndPublish(model, variantIds, parentUnique);
}

/**
* Updates and publishes an existing Document in a single operation
* @param {UmbDocumentDetailModel} model - The Document to update
* @param {Array<UmbVariantId>} variantIds - The variants to publish after updating
* @returns {*}
* @memberof UmbDocumentDetailRepository
*/
async updateAndPublish(model: UmbDocumentDetailModel, variantIds: Array<UmbVariantId>) {
return this.detailDataSource.updateAndPublish(model, variantIds);
}
}

export { UmbDocumentDetailRepository as api };
Loading