From 3d3449de6bf2462eceb65693eff85c6e3941dba7 Mon Sep 17 00:00:00 2001 From: Mykola Shlyakhtun Date: Thu, 9 Jan 2025 15:10:58 +0300 Subject: [PATCH] Related keyphrase status neutral/orange instead of happy/green when all the results are good #4475 Remove related keyword score aggregator and move filter into base SEOScoreAggregator --- .../RelatedKeywordScoreAggregatorSpec.js | 127 ------------------ .../SEOScoreAggregatorSpec.js | 12 ++ .../RelatedKeywordScoreAggregator.js | 35 ----- .../scoreAggregators/SEOScoreAggregator.js | 20 ++- .../yoastseo/src/worker/AnalysisWebWorker.js | 5 +- 5 files changed, 31 insertions(+), 168 deletions(-) delete mode 100644 packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js delete mode 100644 packages/yoastseo/src/scoring/scoreAggregators/RelatedKeywordScoreAggregator.js diff --git a/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js b/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js deleted file mode 100644 index bf51a8c521a..00000000000 --- a/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js +++ /dev/null @@ -1,127 +0,0 @@ -import { RelatedKeywordScoreAggregator } from "../../../src/scoring/scoreAggregators"; -import AssessmentResult from "../../../src/values/AssessmentResult"; - -describe( "RelatedKeywordScoreAggregator", () => { - let aggregator; - - beforeEach( () => { - aggregator = new RelatedKeywordScoreAggregator(); - } ); - - describe( "aggregate", () => { - it( "returns score 0 with default assessment results", () => { - const results = [ - new AssessmentResult(), - new AssessmentResult(), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 0 ); - } ); - - it( "returns score 0 without assessment results", () => { - const results = []; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 0 ); - } ); - - it( "returns score 100 with only score 9 assessment results", () => { - const results = [ - new AssessmentResult( { score: 9 } ), - new AssessmentResult( { score: 9 } ), - new AssessmentResult( { score: 9 } ), - new AssessmentResult( { score: 9 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 100 ); - } ); - - it( "returns score 89 with only score 8 assessment results", () => { - const results = [ - new AssessmentResult( { score: 8 } ), - new AssessmentResult( { score: 8 } ), - new AssessmentResult( { score: 8 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 89 ); - } ); - - it( "returns score 67 with only score 6 assessment results", () => { - const results = [ - new AssessmentResult( { score: 6 } ), - new AssessmentResult( { score: 6 } ), - new AssessmentResult( { score: 6 } ), - new AssessmentResult( { score: 6 } ), - new AssessmentResult( { score: 6 } ), - new AssessmentResult( { score: 6 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 67 ); - } ); - - it( "returns score 33 with only score 3 assessment results", () => { - const results = [ - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 3 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 33 ); - } ); - - it( "returns score 11 with only score 1 assessment results", () => { - const results = [ - new AssessmentResult( { score: 1 } ), - new AssessmentResult( { score: 1 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 11 ); - } ); - - it( "returns score as expected with combined assessment results", () => { - const results = [ - new AssessmentResult( { score: 1 } ), - new AssessmentResult( { score: 2 } ), - new AssessmentResult( { score: 3 } ), - new AssessmentResult( { score: 4 } ), - new AssessmentResult( { score: 5 } ), - new AssessmentResult( { score: 6 } ), - new AssessmentResult( { score: 7 } ), - new AssessmentResult( { score: 8 } ), - new AssessmentResult( { score: 9 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 56 ); - } ); - - it( "returns score as expected with combined assessment results - part 2", () => { - const results = [ - new AssessmentResult( { score: 5 } ), - new AssessmentResult( { score: 4 } ), - new AssessmentResult( { score: 8 } ), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 63 ); - } ); - - it( "exclude assessments without score from aggregator", () => { - const results = [ - new AssessmentResult( { score: 5 } ), - new AssessmentResult(), - new AssessmentResult( { score: 4 } ), - new AssessmentResult( { score: 8 } ), - new AssessmentResult(), - ]; - const score = aggregator.aggregate( results ); - expect( score ).toBe( 63 ); - } ); - } ); -} ); diff --git a/packages/yoastseo/spec/scoring/scoreAggregators/SEOScoreAggregatorSpec.js b/packages/yoastseo/spec/scoring/scoreAggregators/SEOScoreAggregatorSpec.js index 6c6809bc907..df15f146642 100644 --- a/packages/yoastseo/spec/scoring/scoreAggregators/SEOScoreAggregatorSpec.js +++ b/packages/yoastseo/spec/scoring/scoreAggregators/SEOScoreAggregatorSpec.js @@ -111,5 +111,17 @@ describe( "SEOScoreAggregator", () => { const score = aggregator.aggregate( results ); expect( score ).toBe( 63 ); } ); + + it( "exclude assessments without score from aggregator", () => { + const results = [ + new AssessmentResult( { score: 5 } ), + new AssessmentResult(), + new AssessmentResult( { score: 4 } ), + new AssessmentResult( { score: 8 } ), + new AssessmentResult(), + ]; + const score = aggregator.aggregate( results ); + expect( score ).toBe( 63 ); + } ); } ); } ); diff --git a/packages/yoastseo/src/scoring/scoreAggregators/RelatedKeywordScoreAggregator.js b/packages/yoastseo/src/scoring/scoreAggregators/RelatedKeywordScoreAggregator.js deleted file mode 100644 index ea30c4b3327..00000000000 --- a/packages/yoastseo/src/scoring/scoreAggregators/RelatedKeywordScoreAggregator.js +++ /dev/null @@ -1,35 +0,0 @@ -import SEOScoreAggregator from "./SEOScoreAggregator"; - -/** - * Aggregates SEO assessment results into a single score. - * - * @memberOf module:parsedPaper/assess - */ -class RelatedKeywordScoreAggregator extends SEOScoreAggregator { - /** - * Returns the list of valid results. - * Valid results are all results that have a score. - * - * @param {AssessmentResult[]} results The results to filter the valid results from. - * - * @returns {AssessmentResult[]} The list of valid results. - */ - getValidResults( results ) { - return results.filter( result => result.hasScore() ); - } - - /** - * Aggregates the given assessment results into a single score. - * - * @param {AssessmentResult[]} results The assessment results. - * - * @returns {number} The aggregated score. - */ - aggregate( results ) { - const validResults = this.getValidResults( results ); - - return super.aggregate( validResults ); - } -} - -export default RelatedKeywordScoreAggregator; diff --git a/packages/yoastseo/src/scoring/scoreAggregators/SEOScoreAggregator.js b/packages/yoastseo/src/scoring/scoreAggregators/SEOScoreAggregator.js index 12da6c65f5c..e2956e68798 100644 --- a/packages/yoastseo/src/scoring/scoreAggregators/SEOScoreAggregator.js +++ b/packages/yoastseo/src/scoring/scoreAggregators/SEOScoreAggregator.js @@ -32,6 +32,18 @@ const ScoreFactor = 9; * @memberOf module:parsedPaper/assess */ class SEOScoreAggregator extends ScoreAggregator { + /** + * Returns the list of valid results. + * Valid results are all results that have a score. + * + * @param {AssessmentResult[]} results The results to filter the valid results from. + * + * @returns {AssessmentResult[]} The list of valid results. + */ + getValidResults( results ) { + return results.filter( result => result.hasScore() ); + } + /** * Aggregates the given assessment results into a single score. * @@ -40,14 +52,16 @@ class SEOScoreAggregator extends ScoreAggregator { * @returns {number} The aggregated score. */ aggregate( results ) { - const score = results.reduce( ( sum, result ) => sum + result.getScore(), 0 ); + const validResults = this.getValidResults( results ); + + const score = validResults.reduce( ( sum, result ) => sum + result.getScore(), 0 ); /* * Whenever the divide by part is 0 this can result in a `NaN` value. Then 0 should be returned as fallback. - * This seemed better than the if check `results.length === 0`, + * This seemed better than the if check `validResults.length === 0`, * because it also protects against ScoreFactor being 0. */ - return Math.round( ( score * ScoreScale ) / ( results.length * ScoreFactor ) ) || 0; + return Math.round( ( score * ScoreScale ) / ( validResults.length * ScoreFactor ) ) || 0; } } diff --git a/packages/yoastseo/src/worker/AnalysisWebWorker.js b/packages/yoastseo/src/worker/AnalysisWebWorker.js index 452a69dfd74..1cc7f122105 100644 --- a/packages/yoastseo/src/worker/AnalysisWebWorker.js +++ b/packages/yoastseo/src/worker/AnalysisWebWorker.js @@ -30,7 +30,7 @@ import SEOAssessor from "../scoring/assessors/seoAssessor.js"; import TaxonomyAssessor from "../scoring/assessors/taxonomyAssessor.js"; // Tree assessor functionality. -import { ReadabilityScoreAggregator, SEOScoreAggregator, RelatedKeywordScoreAggregator } from "../scoring/scoreAggregators"; +import { ReadabilityScoreAggregator, SEOScoreAggregator } from "../scoring/scoreAggregators"; const logger = getLogger( "yoast-analysis-worker" ); logger.setDefaultLevel( "error" ); @@ -313,7 +313,6 @@ export default class AnalysisWebWorker { // Score aggregators this._seoScoreAggregator = new SEOScoreAggregator(); this._contentScoreAggregator = new ReadabilityScoreAggregator(); - this._relatedKeywordScoreAggregator = new RelatedKeywordScoreAggregator(); // Tree representation of text to analyze this._tree = null; @@ -1248,7 +1247,7 @@ export default class AnalysisWebWorker { const analysisCombination = { oldAssessor: this._relatedKeywordAssessor, treeAssessor: this._relatedKeywordTreeAssessor, - scoreAggregator: this._relatedKeywordScoreAggregator, + scoreAggregator: this._seoScoreAggregator, }; // We need to remember the key, since the SEO results are stored in an object, not an array.