Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
37 changes: 33 additions & 4 deletions src/controllers/suggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ import { grantSuggestionsForOpportunity } from '../support/grant-suggestions-han

const VALIDATION_ERROR_NAME = 'ValidationError';

// Maximum number of CWV suggestions returned to PLG (sites-optimizer-ui) clients.
// The audit-worker already stores only failing-metric pages sorted by page views desc,
// so slicing to this limit gives the top N most-viewed pages with CWV issues.
const CWV_PLG_SUGGESTION_LIMIT = 3;

const GEO_EXPERIMENT_SCHEDULE = Object.freeze({
PRE_CRON_EXPRESSION: '0 * * * *', // hourly (fires immediately via triggerImmediately: true)
// 5 minutes — only needs to live long enough for the immediate trigger
Expand Down Expand Up @@ -223,6 +228,26 @@ function SuggestionsController(ctx, sqs, env) {
}
};

/**
* For PLG (sites-optimizer-ui) clients, limits CWV suggestions to the top N entries.
* The suggestions are already ordered by page views descending from the audit-worker,
* so slicing gives the highest-traffic pages with failing metrics.
* Returns all suggestions unchanged for non-CWV types, non-PLG clients, or sites
* that do not have the summit-plg handler enabled.
* @param {Object} site - Site entity.
* @param {Array} entities - Suggestion entities to limit.
* @param {Object} opportunity - Opportunity entity (may be null if no suggestions).
* @param {Object} context - Request context.
* @returns {Promise<Array>} Limited suggestion entities.
*/
const applyPlgCwvLimit = async (site, entities, opportunity, context) => {
if (opportunity?.getType() !== 'cwv') return entities;
if (context.pathInfo?.headers?.['x-client-type'] !== 'sites-optimizer-ui') return entities;
const configuration = await Configuration.findLatest();
if (!configuration?.isHandlerEnabledForSite('summit-plg', site)) return entities;
return entities.slice(0, CWV_PLG_SUGGESTION_LIMIT);
};

/**
* Gets all suggestions for a given site and opportunity
* @param {Object} context of the request
Expand Down Expand Up @@ -286,7 +311,8 @@ function SuggestionsController(ctx, sqs, env) {
);
}
const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context);
const suggestions = grantedEntities.map(
const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context);
const suggestions = limitedEntities.map(
(sugg) => SuggestionDto.toJSON(sugg, view, opportunity),
);
return ok(suggestions);
Expand Down Expand Up @@ -345,7 +371,8 @@ function SuggestionsController(ctx, sqs, env) {
}
}
const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context);
const suggestions = grantedEntities.map(
const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context);
const suggestions = limitedEntities.map(
(sugg) => SuggestionDto.toJSON(sugg, view, opportunity),
);

Expand Down Expand Up @@ -401,7 +428,8 @@ function SuggestionsController(ctx, sqs, env) {
}
}
const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context);
const suggestions = grantedEntities.map(
const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context);
const suggestions = limitedEntities.map(
(sugg) => SuggestionDto.toJSON(sugg, view, opportunity),
);
return ok(suggestions);
Expand Down Expand Up @@ -460,7 +488,8 @@ function SuggestionsController(ctx, sqs, env) {
}
}
const grantedEntities = await filterByGrantStatus(site, suggestionEntities, context);
const suggestions = grantedEntities.map(
const limitedEntities = await applyPlgCwvLimit(site, grantedEntities, opportunity, context);
const suggestions = limitedEntities.map(
(sugg) => SuggestionDto.toJSON(sugg, view, opportunity),
);
return ok({
Expand Down
98 changes: 98 additions & 0 deletions test/controllers/suggestions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,104 @@ describe('Suggestions Controller', () => {
expect(mockLog.warn).to.have.been.calledWith('Grant suggestions handler failed', 'grant failure');
});

it('limits CWV suggestions to top 3 for PLG (sites-optimizer-ui) clients', async () => {
const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({
getId() { return id; },
getOpportunityId() { return OPPORTUNITY_ID; },
getType() { return 'CODE_CHANGE'; },
getStatus() { return 'NEW'; },
getRank() { return i; },
getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; },
getKpiDeltas() { return null; },
getCreatedAt() { return '2024-01-01T00:00:00.000Z'; },
getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; },
getSkipReason() { return null; },
getSkipDetail() { return null; },
getUpdatedBy() { return 'system'; },
getOpportunity() {
return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } };
},
}));

mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs);

// mockConfiguration already has isHandlerEnabledForSite('summit-plg', site) = true
const response = await suggestionsController.getAllForOpportunity({
params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID },
pathInfo: { headers: { 'x-client-type': 'sites-optimizer-ui' } },
...context,
});
expect(response.status).to.equal(200);
const result = await response.json();
expect(result).to.be.an('array').with.lengthOf(3);
});

it('returns all CWV suggestions when summit-plg handler is not enabled for the site', async () => {
const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({
getId() { return id; },
getOpportunityId() { return OPPORTUNITY_ID; },
getType() { return 'CODE_CHANGE'; },
getStatus() { return 'NEW'; },
getRank() { return i; },
getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; },
getKpiDeltas() { return null; },
getCreatedAt() { return '2024-01-01T00:00:00.000Z'; },
getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; },
getSkipReason() { return null; },
getSkipDetail() { return null; },
getUpdatedBy() { return 'system'; },
getOpportunity() {
return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } };
},
}));

mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs);
// Override configuration to return false for summit-plg
mockSuggestionDataAccess.Configuration.findLatest.resolves({
isHandlerEnabledForSite: () => false,
});

const response = await suggestionsController.getAllForOpportunity({
params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID },
pathInfo: { headers: { 'x-client-type': 'sites-optimizer-ui' } },
...context,
});
expect(response.status).to.equal(200);
const result = await response.json();
expect(result).to.be.an('array').with.lengthOf(4);
});

it('returns all CWV suggestions when x-client-type is not sites-optimizer-ui', async () => {
const cwvSuggs = ['id-0', 'id-1', 'id-2', 'id-3'].map((id, i) => ({
getId() { return id; },
getOpportunityId() { return OPPORTUNITY_ID; },
getType() { return 'CODE_CHANGE'; },
getStatus() { return 'NEW'; },
getRank() { return i; },
getData() { return { type: 'url', url: `https://example.com/page${i}`, jiraLink: '' }; },
getKpiDeltas() { return null; },
getCreatedAt() { return '2024-01-01T00:00:00.000Z'; },
getUpdatedAt() { return '2024-01-01T00:00:00.000Z'; },
getSkipReason() { return null; },
getSkipDetail() { return null; },
getUpdatedBy() { return 'system'; },
getOpportunity() {
return { getSiteId() { return SITE_ID; }, getType() { return 'cwv'; } };
},
}));

mockSuggestionDataAccess.Suggestion.allByOpportunityId.resolves(cwvSuggs);

// No sites-optimizer-ui header → all 4 returned
const response = await suggestionsController.getAllForOpportunity({
params: { siteId: SITE_ID, opportunityId: OPPORTUNITY_ID },
...context,
});
expect(response.status).to.equal(200);
const result = await response.json();
expect(result).to.be.an('array').with.lengthOf(4);
});

it('gets all suggestions for an opportunity and a site for non belonging to the organization', async () => {
sandbox.stub(AccessControlUtil.prototype, 'hasAccess').returns(false);
sandbox.stub(context.attributes.authInfo, 'hasOrganization').returns(false);
Expand Down
Loading