From 3b25bec397dd2d573b93f8745149fd454b471a58 Mon Sep 17 00:00:00 2001 From: Mykola Shlyakhtun Date: Thu, 2 Jan 2025 12:54:55 +0300 Subject: [PATCH 1/3] Related keyphrase status neutral/orange instead of happy/green when all the results are good #4475 --- .../RelatedKeywordScoreAggregatorSpec.js | 24 +++++++++++++ .../RelatedKeywordScoreAggregator.js | 35 +++++++++++++++++++ .../src/scoring/scoreAggregators/index.js | 1 + .../yoastseo/src/worker/AnalysisWebWorker.js | 5 +-- 4 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js create 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 new file mode 100644 index 00000000000..23ff7a4c8b8 --- /dev/null +++ b/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js @@ -0,0 +1,24 @@ +import { RelatedKeywordScoreAggregator } from "../../../src/scoring/scoreAggregators"; +import AssessmentResult from "../../../src/values/AssessmentResult"; + +describe( "RelatedKeywordScoreAggregator", () => { + let aggregator; + + beforeEach( () => { + aggregator = new RelatedKeywordScoreAggregator(); + } ); + + describe( "aggregate", () => { + 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 new file mode 100644 index 00000000000..ea30c4b3327 --- /dev/null +++ b/packages/yoastseo/src/scoring/scoreAggregators/RelatedKeywordScoreAggregator.js @@ -0,0 +1,35 @@ +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/index.js b/packages/yoastseo/src/scoring/scoreAggregators/index.js index 8f0e70f37b6..2c5e11eedb0 100644 --- a/packages/yoastseo/src/scoring/scoreAggregators/index.js +++ b/packages/yoastseo/src/scoring/scoreAggregators/index.js @@ -1,3 +1,4 @@ export { default as ReadabilityScoreAggregator } from "./ReadabilityScoreAggregator"; +export { default as RelatedKeywordScoreAggregator } from "./RelatedKeywordScoreAggregator"; export { default as ScoreAggregator } from "./ScoreAggregator"; export { default as SEOScoreAggregator } from "./SEOScoreAggregator"; diff --git a/packages/yoastseo/src/worker/AnalysisWebWorker.js b/packages/yoastseo/src/worker/AnalysisWebWorker.js index 42efb800333..f077e17495f 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 } from "../scoring/scoreAggregators"; +import { ReadabilityScoreAggregator, SEOScoreAggregator, RelatedKeywordScoreAggregator } from "../scoring/scoreAggregators"; const logger = getLogger( "yoast-analysis-worker" ); logger.setDefaultLevel( "error" ); @@ -313,6 +313,7 @@ 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; @@ -1241,7 +1242,7 @@ export default class AnalysisWebWorker { const analysisCombination = { oldAssessor: this._relatedKeywordAssessor, treeAssessor: this._relatedKeywordTreeAssessor, - scoreAggregator: this._seoScoreAggregator, + scoreAggregator: this._relatedKeywordScoreAggregator, }; // We need to remember the key, since the SEO results are stored in an object, not an array. From 1aed651eb84f8f0c196a5db482f056bdb023f24f Mon Sep 17 00:00:00 2001 From: Mykola Shlyakhtun Date: Thu, 2 Jan 2025 12:56:19 +0300 Subject: [PATCH 2/3] Related keyphrase status neutral/orange instead of happy/green when all the results are good #4475 Duplicate test from SEOScoreAggregator --- .../RelatedKeywordScoreAggregatorSpec.js | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js b/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js index 23ff7a4c8b8..bf51a8c521a 100644 --- a/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js +++ b/packages/yoastseo/spec/scoring/scoreAggregators/RelatedKeywordScoreAggregatorSpec.js @@ -9,6 +9,109 @@ describe( "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 } ), From 3d3449de6bf2462eceb65693eff85c6e3941dba7 Mon Sep 17 00:00:00 2001 From: Mykola Shlyakhtun Date: Thu, 9 Jan 2025 15:10:58 +0300 Subject: [PATCH 3/3] 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.