Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
26 changes: 26 additions & 0 deletions docs/openapi/schemas.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4301,6 +4301,15 @@ FixCreate:
description: Optional array of suggestion IDs to associate with this fix
items:
$ref: '#/Id'
suggestionsTargetStatus:
$ref: '#/SuggestionStatus'
description: >-
Optional. When set, the suggestions in suggestionIds are atomically
transitioned to this status once this fix is successfully created and linked.
Required for opportunity types where a suggestion cannot be marked FIXED
without a corresponding Fix entity (see PATCH suggestion status endpoints,
which reject direct transitions to FIXED for those types). Omit to leave
suggestion status untouched.

FixOperationSuccess:
type: object
Expand All @@ -4315,6 +4324,15 @@ FixOperationSuccess:
type: integer
fix:
$ref: '#/Fix'
suggestions:
type: array
description: >-
Present only when suggestionsTargetStatus was provided in the request for
this fix and linked suggestions were transitioned as a result. Reflects
each suggestion's post-transition state, so the caller can sync local state
without a separate fetch. Absent when suggestionsTargetStatus was not set.
items:
$ref: '#/Suggestion'
required:
- index
- statusCode
Expand Down Expand Up @@ -4423,6 +4441,14 @@ FixStatusUpdate:
status:
description: Status of this fix; status reflects overall fix execution, flagged here for optimization
$ref: '#/SuggestionStatus'
suggestionsTargetStatus:
$ref: '#/SuggestionStatus'
description: >-
Optional. When set, the fix's linked suggestions are atomically
transitioned to this status once the fix status update is successfully
persisted. Required for opportunity types where a suggestion cannot be
marked FIXED without a corresponding Fix entity. Omit to leave
suggestion status untouched.
required:
- id
- status
Expand Down
28 changes: 26 additions & 2 deletions docs/openapi/site-opportunities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,12 @@ site-opportunity-suggestions-status:
call this endpoint. The admin-only gate for `REJECTED` status transitions is bypassed
for S2S callers holding this capability; all other validations (site membership,
opportunity lookup, valid status transition) apply unchanged.

**FIXED status:** for opportunity types where a suggestion must have a corresponding
Fix entity before it can be FIXED, a direct transition to FIXED via this endpoint is
rejected with a 400. Use `POST .../opportunities/{opportunityId}/fixes` with
`suggestionsTargetStatus: FIXED` instead, which creates the Fix and transitions
the suggestion atomically.
tags:
- opportunity-suggestions
requestBody:
Expand Down Expand Up @@ -1375,6 +1381,12 @@ site-opportunity-suggestion:
operationId: updateSiteOpportunitySuggestion
summary: |
Update specific attributes of an existing suggestion
description: |
For opportunity types where a suggestion must have a corresponding Fix entity
before it can be FIXED, a direct transition to FIXED via this endpoint is
rejected with a 400. Use `POST .../opportunities/{opportunityId}/fixes` with
`suggestionsTargetStatus: FIXED` instead, which creates the Fix and transitions
the suggestion atomically.
tags:
- opportunity-suggestions
requestBody:
Expand Down Expand Up @@ -1800,9 +1812,15 @@ site-opportunity-fixes:
summary: |
Create and add a list of one or more fixes to an opportunity in one transaction
description: |
Creates one or more fix entities for a given opportunity. Each fix can optionally include
an array of suggestionIds to associate existing suggestions with the fix. This allows you
Creates one or more fix entities for a given opportunity. Each fix can optionally include
an array of suggestionIds to associate existing suggestions with the fix. This allows you
to link fixes to the suggestions they address in a single transaction.

Set `suggestionsTargetStatus` on a fix to also transition its linked
suggestions to that status once the fix is successfully created and linked —
atomically, so a suggestion is never marked FIXED without a persisted Fix entity.
This is required for opportunity types where the suggestion-status PATCH
endpoints reject a direct transition to FIXED.
tags:
- opportunity-fixes
requestBody:
Expand Down Expand Up @@ -2042,6 +2060,12 @@ site-opportunity-status:
operationId: updateSiteOpportunityStatus
summary: |
Update the status of one or multiple fixes in one transaction
description: |
Set `suggestionsTargetStatus` on a fix update to also transition its linked
suggestions to that status once the fix status update is successfully
persisted — atomically, so a suggestion is never transitioned independently of
the fix update. This is required for opportunity types where the
suggestion-status PATCH endpoints reject a direct transition to FIXED.
tags:
- opportunity-fixes
requestBody:
Expand Down
79 changes: 72 additions & 7 deletions src/controllers/fixes.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,23 @@ export class FixesController {
return dedupedResult;
}

// Resolve and validate suggestionIds before creating the fix, so an invalid
// suggestion ID (unknown, or belonging to a different opportunity) fails fast
// without leaving behind an orphaned, unlinked FixEntity.
let suggestions;
if (fixData.suggestionIds) {
suggestions = await Promise.all(
fixData.suggestionIds.map((id) => this.#Suggestion.findById(id)),
);
if (suggestions.some((s) => !s || s.getOpportunityId() !== opportunityId)) {
return {
index,
message: 'Invalid suggestion IDs',
statusCode: 400,
};
}
}

const enrichedFixData = await FixesController.#enrichWithDocumentPath(
fixData,
enrichmentCtx,
Expand All @@ -438,15 +455,32 @@ export class FixesController {
opportunityId,
...(hasText(callerUserId) && { executedBy: callerUserId }),
});
if (fixData.suggestionIds) {
const suggestions = await Promise.all(
fixData.suggestionIds.map((id) => this.#Suggestion.findById(id)),
);
let updatedSuggestions;
if (suggestions) {
await FixEntity.setSuggestionsForFixEntity(opportunityId, fixEntity, suggestions);

// Opt-in (SITES-fix-orphan): the fix has just been persisted and linked, so
// it's safe to transition these suggestions to the caller-specified status
// atomically with fix creation. Only reached after
// FixEntity.create/setSuggestionsForFixEntity succeeded above; a failure
// there throws before this line, so a suggestion is never transitioned
// without a persisted, linked fix.
if (hasText(fixData.suggestionsTargetStatus)) {
updatedSuggestions = await this.#Suggestion.bulkUpdateStatus(
suggestions,
fixData.suggestionsTargetStatus,
);
}
}
return {
index,
fix: FixDto.toJSON(fixEntity),
// Only present when suggestionsTargetStatus was provided — callers that
// didn't mutate suggestion status get no suggestions field, since nothing
// about suggestion state changed for them to sync.
...(updatedSuggestions && {
suggestions: updatedSuggestions.map((s) => SuggestionDto.toJSON(s)),
}),
statusCode: 201,
};
} catch (error) {
Expand Down Expand Up @@ -652,7 +686,14 @@ export class FixesController {

const fixes = await Promise.all(
context.data.map(
(data, index) => this.#patchFixStatus(data.id, data.status, index, opportunityId, siteId),
(data, index) => this.#patchFixStatus(
data.id,
data.status,
index,
opportunityId,
siteId,
data.suggestionsTargetStatus,
),
),
);
const succeeded = countSucceeded(fixes);
Expand All @@ -662,7 +703,7 @@ export class FixesController {
}, 207);
}

async #patchFixStatus(uuid, status, index, opportunityId, siteId) {
async #patchFixStatus(uuid, status, index, opportunityId, siteId, suggestionsTargetStatus) {
if (!hasText(uuid)) {
return {
index,
Expand Down Expand Up @@ -701,8 +742,32 @@ export class FixesController {
}

fix.setStatus(status);
const updatedFix = await fix.save();

// Opt-in (mirrors createFixes' suggestionsTargetStatus): only reached after the
// fix status write above succeeded, so a suggestion is never transitioned
// without a persisted fix status update.
let updatedSuggestions;
if (hasText(suggestionsTargetStatus)) {
const suggestions = await this.#FixEntity.getSuggestionsByFixEntityId(uuid);
if (Array.isArray(suggestions) && suggestions.length > 0) {
updatedSuggestions = await this.#Suggestion.bulkUpdateStatus(
suggestions,
suggestionsTargetStatus,
);
}
}

return {
index, uuid, fix: FixDto.toJSON(await fix.save()), statusCode: 200,
index,
uuid,
fix: FixDto.toJSON(updatedFix),
// Only present when suggestionsTargetStatus was provided and linked
// suggestions actually existed to transition.
...(updatedSuggestions && {
suggestions: updatedSuggestions.map((s) => SuggestionDto.toJSON(s)),
}),
statusCode: 200,
};
} catch (error) {
const statusCode = error?.name === VALIDATION_ERROR_NAME ? /* c8 ignore next */ 400 : 500;
Expand Down
24 changes: 24 additions & 0 deletions src/controllers/suggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
isImpactMeasurementEligible,
} from '../support/geo-experiment-helper.js';
import { FixDto } from '../dto/fix.js';
import { SUGGESTION_TYPES_REQUIRING_FIX_ENTITY } from '../utils/suggestion-fix-required-types.js';
import { GeoExperimentDto } from '../dto/geo-experiment.js';
import {
sendAutofixMessage,
Expand Down Expand Up @@ -1149,6 +1150,12 @@ function SuggestionsController(ctx, sqs, env) {

let isNewSkipTransition = false;
if (hasText(status) && status !== suggestion.getStatus()) {
if (
status === SuggestionModel.STATUSES.FIXED
&& SUGGESTION_TYPES_REQUIRING_FIX_ENTITY.includes(opportunity.getType())
) {
return badRequest(`Suggestions of type '${opportunity.getType()}' cannot be marked FIXED directly; create a Fix via POST /sites/:siteId/opportunities/:opportunityId/fixes with suggestionsTargetStatus: FIXED instead.`);
}
const { valid, error } = validateSkipFields(status, skipReason, skipDetail);
if (!valid) {
return badRequest(error);
Expand Down Expand Up @@ -1368,6 +1375,23 @@ function SuggestionsController(ctx, sqs, env) {
}
}

// FIXED requires a Fix entity for these types (data-integrity gate): reject
// a direct transition to FIXED via this generic status endpoint and point
// callers at the fix-creation endpoint, which creates the Fix and flips
// suggestion status atomically. Checked ahead of the general transition
// gate below for the same reason the REJECTED rule is.
if (
status === SuggestionModel.STATUSES.FIXED
&& SUGGESTION_TYPES_REQUIRING_FIX_ENTITY.includes(opportunity.getType())
) {
return {
index,
uuid: id,
message: `Suggestions of type '${opportunity.getType()}' cannot be marked FIXED directly; create a Fix via POST /sites/:siteId/opportunities/:opportunityId/fixes with suggestionsTargetStatus: FIXED instead.`,
statusCode: 400,
};
}

// Per-item transition-legality gate (SITES-49063; part of the SITES-47286
// warn->enforce rollout). NOTE: the REJECTED hard-rule above is a stricter
// subset of this and fires first — keep it ahead of this general gate.
Expand Down
48 changes: 48 additions & 0 deletions src/utils/suggestion-fix-required-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2026 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

/**
* Opportunity types whose suggestions must never be transitioned to FIXED via the
* plain suggestion-status PATCH endpoints (`PATCH .../suggestions/:suggestionId` or
* `PATCH .../suggestions/status`). For these types a FixEntity is expected to exist
* before a suggestion is FIXED, so the transition must go through
* `POST .../opportunities/:opportunityId/fixes` (with `markSuggestionsFixed: true`)
* instead, which creates the FixEntity and flips the suggestion status atomically.
*
* `generic-opportunity` is deliberately excluded: it's a shared fallback type used by
* several unrelated flows (cwv-trends, accessibility reports, hreflang/canonical paid
* fallback), not a single semantic opportunity type, so blocking it would over-restrict
* unrelated suggestions.
*/
export const SUGGESTION_TYPES_REQUIRING_FIX_ENTITY = [
'cwv',
'alt-text',
'broken-backlinks',
'broken-internal-links',
'security-vulnerabilities',
'security-permissions',
'security-permissions-redundant',
'security-csp',
'structured-data',
'meta-tags',
'hreflang',
'redirect-chains',
'sitemap',
'canonical',
'accessibility',
'a11y-color-contrast',
'form-accessibility',
'high-organic-low-ctr',
'high-form-views-low-conversions',
'high-page-views-low-form-nav',
'high-page-views-low-form-views',
];
Loading
Loading