Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes the overall score for related keyphrase and collection assessors #21941

Merged
merged 18 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3b25bec
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 2, 2025
1aed651
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 2, 2025
1241e1e
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 9, 2025
3d3449d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 9, 2025
a808333
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 9, 2025
b6a6d7d
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 20, 2025
e26339c
Revert "Related keyphrase status neutral/orange instead of happy/gree…
mykola Jan 20, 2025
bf7f76d
Revert "Related keyphrase status neutral/orange instead of happy/gree…
mykola Jan 20, 2025
aa3f33e
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 24, 2025
a2b684d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 24, 2025
9205a1b
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 24, 2025
75f717c
Merge remote-tracking branch 'origin/trunk' into 4475-fix-related-key…
mykola Jan 28, 2025
9856a0d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 28, 2025
9800b9d
Related keyphrase status neutral/orange instead of happy/green when a…
mykola Jan 28, 2025
2e06b9a
Makes the unit tests more specific
mhkuu Jan 31, 2025
e6e1d6e
Do not set a default aggregator, set it at the right places, and clea…
mhkuu Jan 31, 2025
d95b5ef
Improves JSDoc for the score aggregators
mhkuu Jan 31, 2025
a056131
Additional cleanup for the AnalysisWebWorker
mhkuu Jan 31, 2025
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
2 changes: 1 addition & 1 deletion packages/yoastseo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"clean": "rm -rf build types",
"pretest": "grunt get-premium-configuration",
"test": "jest",
"lint": "eslint . --max-warnings 26"
"lint": "eslint . --max-warnings 21"
},
"engines": {
"node": ">=8.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,5 +111,17 @@ describe( "SEOScoreAggregator", () => {
const score = aggregator.aggregate( results );
expect( score ).toBe( 63 );
} );

it( "does not 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( 38 );
} );
} );
} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { ValidOnlyResultsScoreAggregator } from "../../../src/scoring/scoreAggregators";
import AssessmentResult from "../../../src/values/AssessmentResult";

describe( "ValidOnlyResultsScoreAggregator", () => {
let aggregator;

beforeEach( () => {
aggregator = new ValidOnlyResultsScoreAggregator();
} );

describe( "aggregate", () => {
it( "excludes 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 );
} );
} );
} );
76 changes: 0 additions & 76 deletions packages/yoastseo/spec/worker/AnalysisWebWorkerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ import { SEOScoreAggregator } from "../../src/scoring/scoreAggregators";
import { TreeResearcher } from "../../src/parsedPaper/research";
import AssessmentResult from "../../src/values/AssessmentResult.js";
import Paper from "../../src/values/Paper.js";
import InvalidTypeError from "../../src/errors/invalidType.js";
import { StructuredNode } from "../../src/parsedPaper/structure/tree";


// Full-length texts to test
import testTexts from "../fullTextTests/testTexts";
Expand Down Expand Up @@ -1932,79 +1929,6 @@ describe( "AnalysisWebWorker", () => {
} );
} );

describe( "registerParser", () => {
/**
* A mock parser.
*/
class MockParser {
/**
* Checks if this parser is applicable.
*
* @returns {boolean} Whether the parser is applicable.
*/
isApplicable() {
return true;
}

/**
* Parses the paper.
*
* @returns {module:parsedPaper/structure.StructuredNode} The tree structure.
*/
parse() {
return new StructuredNode( "some-tag" );
}
}

beforeEach( () => {
scope = createScope();
worker = new AnalysisWebWorker( scope, researcher );
} );

it( "can register a custom parser, if it is class-based and has the appropriate methods.", () => {
const mockParser = new MockParser();

worker.registerParser( mockParser );

expect( worker._registeredParsers ).toHaveLength( 1 );
expect( worker._registeredParsers[ 0 ] ).toEqual( mockParser );
} );

it( "can register a custom parser, if it has an `isApplicable` and a `parse` method.", () => {
const mockParser = {
isApplicable: () => true,
parse: () => new StructuredNode( "Hello!" ),
};

worker.registerParser( mockParser );

expect( worker._registeredParsers ).toHaveLength( 1 );
expect( worker._registeredParsers[ 0 ] ).toEqual( mockParser );
} );

it( "throws an error when registering a custom parser, if it does not have an `isApplicable` method.", () => {
const mockParser = {
parse: () => new StructuredNode( "Hello!" ),
};

expect( () => worker.registerParser( mockParser ) ).toThrow( InvalidTypeError );
} );

it( "throws an error when registering a custom parser, if it does not have a `parse` method.", () => {
const mockParser = {
isApplicable: () => true,
};

expect( () => worker.registerParser( mockParser ) ).toThrow( InvalidTypeError );
} );

it( "throws an error when registering a custom parser, if it neither has `isApplicable` method nor `parse` method.", () => {
const mockParser = {};

expect( () => worker.registerParser( mockParser ) ).toThrow( InvalidTypeError );
} );
} );

describe( "set assessors", () => {
const customAssessorOptions = { url: "url" };
beforeEach( () => {
Expand Down
77 changes: 48 additions & 29 deletions packages/yoastseo/src/scoring/assessors/assessor.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// External dependencies.
import { __, sprintf } from "@wordpress/i18n";
import { filter, find, findIndex, isFunction, isUndefined, map } from "lodash";
import { isFunction, isUndefined } from "lodash";

// Internal dependencies.
import AssessmentResult from "../../values/AssessmentResult.js";
Expand All @@ -25,9 +25,33 @@ class Assessor {
constructor( researcher, options ) {
this.type = "assessor";
this.setResearcher( researcher );

/**
* The list of assessments.
* @type {Assessment[]}
* @private
*/
this._assessments = [];

/**
* The list of results.
* @type {AssessmentResult[]}
*/
this.results = [];

/**
* The options.
* @type {Object|{}}
* @private
*/
this._options = options || {};

/**
* The ScoreAggregator for this assessor.
* @type {ScoreAggregator}
* @private
*/
this._scoreAggregator = null;
}

/**
Expand Down Expand Up @@ -62,11 +86,7 @@ class Assessor {
* @returns {boolean} Whether or not the Assessment is applicable.
*/
isApplicable( assessment, paper, researcher ) {
if ( assessment.hasOwnProperty( "isApplicable" ) || typeof assessment.isApplicable === "function" ) {
return assessment.isApplicable( paper, researcher );
}

return true;
return assessment.isApplicable( paper, researcher );
}

/**
Expand All @@ -76,7 +96,7 @@ class Assessor {
* @returns {boolean} Whether or not the assessment has a marker.
*/
hasMarker( assessment ) {
return isFunction( this._options.marker ) && ( assessment.hasOwnProperty( "getMarks" ) || typeof assessment.getMarks === "function" );
return isFunction( this._options.marker ) && ( Object.hasOwn( assessment, "getMarks" ) || typeof assessment.getMarks === "function" );
}

/**
Expand Down Expand Up @@ -130,14 +150,11 @@ class Assessor {
paper.setTree( build( paper, languageProcessor, shortcodes ) );

let assessments = this.getAvailableAssessments();
this.results = [];

assessments = filter( assessments, function( assessment ) {
return this.isApplicable( assessment, paper, this._researcher );
}.bind( this ) );
assessments = assessments.filter( assessment => this.isApplicable( assessment, paper, this._researcher ) );

this.setHasMarkers( false );
this.results = map( assessments, this.executeAssessment.bind( this, paper, this._researcher ) );
this.results = assessments.map( assessment => this.executeAssessment( paper, this._researcher, assessment ) );

this._lastPaper = paper;
}
Expand Down Expand Up @@ -208,9 +225,7 @@ class Assessor {
* @returns {AssessmentResult[]} The array with all the valid assessments.
*/
getValidResults() {
return filter( this.results, function( result ) {
return this.isValidResult( result );
}.bind( this ) );
return this.results.filter( result => this.isValidResult( result ) );
}

/**
Expand Down Expand Up @@ -245,7 +260,7 @@ class Assessor {
* @returns {boolean} Whether registering the assessment was successful.
*/
addAssessment( name, assessment ) {
if ( ! assessment.hasOwnProperty( "identifier" ) ) {
if ( ! Object.hasOwn( assessment, "identifier" ) ) {
assessment.identifier = name;
}
// If the assessor already has the same assessment, remove it and replace it with the new assessment with the same identifier.
Expand All @@ -264,9 +279,9 @@ class Assessor {
* @returns {void}
*/
removeAssessment( name ) {
const toDelete = findIndex( this._assessments, function( assessment ) {
return assessment.hasOwnProperty( "identifier" ) && name === assessment.identifier;
} );
const toDelete = this._assessments.findIndex( assessment =>
Object.hasOwn( assessment, "identifier" ) && name === assessment.identifier
);

if ( -1 !== toDelete ) {
this._assessments.splice( toDelete, 1 );
Expand All @@ -277,12 +292,12 @@ class Assessor {
* Returns an assessment by identifier
*
* @param {string} identifier The identifier of the assessment.
* @returns {undefined|Assessment} The object if found, otherwise undefined.
* @returns {Assessment} The object if found, otherwise undefined.
*/
getAssessment( identifier ) {
return find( this._assessments, function( assessment ) {
return assessment.hasOwnProperty( "identifier" ) && identifier === assessment.identifier;
} );
return this._assessments.find( assessment =>
Object.hasOwn( assessment, "identifier" ) && identifier === assessment.identifier
);
}

/**
Expand All @@ -292,12 +307,16 @@ class Assessor {
*/
getApplicableAssessments() {
const availableAssessments = this.getAvailableAssessments();
return filter(
availableAssessments,
function( availableAssessment ) {
return this.isApplicable( availableAssessment, this.getPaper(), this._researcher );
}.bind( this )
);
return availableAssessments.filter( assessment => this.isApplicable( assessment, this.getPaper(), this._researcher ) );
}

/**
* Returns the ScoreAggregator for this assessor.
*
* @returns {ScoreAggregator} The specific marker for this assessor.
*/
getScoreAggregator() {
return this._scoreAggregator;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import CollectionSEOAssessor from "../seoAssessor";
import TextLengthAssessment from "../../../assessments/seo/TextLengthAssessment.js";
import { createAnchorOpeningTag } from "../../../../helpers";
import ValidOnlyResultsScoreAggregator from "../../../scoreAggregators/ValidOnlyResultsScoreAggregator";

/**
* The CollectionCornerstoneSEOAssessor class is used for the SEO analysis for cornerstone collections.
Expand All @@ -24,5 +25,7 @@ export default class CollectionCornerstoneSEOAssessor extends CollectionSEOAsses
cornerstoneContent: true,
customContentType: this.type,
} ) );

this._scoreAggregator = new ValidOnlyResultsScoreAggregator();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import PageTitleWidthAssessment from "../../assessments/seo/PageTitleWidthAssess
import SlugKeywordAssessment from "../../assessments/seo/UrlKeywordAssessment.js";
import SingleH1Assessment from "../../assessments/seo/SingleH1Assessment.js";
import { createAnchorOpeningTag } from "../../../helpers";
import ValidOnlyResultsScoreAggregator from "../../scoreAggregators/ValidOnlyResultsScoreAggregator";

/**
* The CollectionSEOAssessor class is used for the SEO analysis for collections.
Expand Down Expand Up @@ -78,5 +79,7 @@ export default class CollectionSEOAssessor extends SEOAssessor {
urlCallToAction: createAnchorOpeningTag( "https://yoa.st/shopify55" ),
} ),
];

this._scoreAggregator = new ValidOnlyResultsScoreAggregator();
}
}
3 changes: 3 additions & 0 deletions packages/yoastseo/src/scoring/assessors/contentAssessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import PassiveVoice from "../assessments/readability/PassiveVoiceAssessment.js";
import SentenceBeginnings from "../assessments/readability/SentenceBeginningsAssessment.js";
import TextPresence from "../assessments/readability/TextPresenceAssessment.js";
import scoreToRating from "../interpreters/scoreToRating.js";
import { ReadabilityScoreAggregator } from "../scoreAggregators";

/**
* The ContentAssessor class is used for the readability analysis.
Expand All @@ -31,6 +32,8 @@ export default class ContentAssessor extends Assessor {
new TextPresence(),
new SentenceBeginnings(),
];

this._scoreAggregator = new ReadabilityScoreAggregator();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import MetaDescriptionKeyword from "../assessments/seo/MetaDescriptionKeywordAss
import TextCompetingLinks from "../assessments/seo/TextCompetingLinksAssessment.js";
import FunctionWordsInKeyphrase from "../assessments/seo/FunctionWordsInKeyphraseAssessment";
import ImageKeyphrase from "../assessments/seo/KeyphraseInImageTextAssessment";
import ValidOnlyResultsScoreAggregator from "../scoreAggregators/ValidOnlyResultsScoreAggregator";

/**
* The relatedKeywordAssessor class is used for the related keyword analysis.
Expand All @@ -29,5 +30,7 @@ export default class RelatedKeywordAssessor extends Assessor {
new FunctionWordsInKeyphrase(),
new ImageKeyphrase(),
];

this._scoreAggregator = new ValidOnlyResultsScoreAggregator();
}
}
3 changes: 3 additions & 0 deletions packages/yoastseo/src/scoring/assessors/seoAssessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import OutboundLinks from "../assessments/seo/OutboundLinksAssessment";
import TitleWidth from "../assessments/seo/PageTitleWidthAssessment";
import FunctionWordsInKeyphrase from "../assessments/seo/FunctionWordsInKeyphraseAssessment";
import SingleH1Assessment from "../assessments/seo/SingleH1Assessment";
import SEOScoreAggregator from "../scoreAggregators/SEOScoreAggregator";

/**
* The SEOAssessor class is used for the general SEO analysis.
Expand Down Expand Up @@ -53,5 +54,7 @@ export default class SEOAssessor extends Assessor {
new FunctionWordsInKeyphrase(),
new SingleH1Assessment(),
];

this._scoreAggregator = new SEOScoreAggregator();
}
}
3 changes: 3 additions & 0 deletions packages/yoastseo/src/scoring/assessors/taxonomyAssessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import PageTitleWidthAssessment from "../assessments/seo/PageTitleWidthAssessmen
import FunctionWordsInKeyphrase from "../assessments/seo/FunctionWordsInKeyphraseAssessment.js";
import SingleH1Assessment from "../assessments/seo/SingleH1Assessment.js";
import { createAnchorOpeningTag } from "../../helpers";
import SEOScoreAggregator from "../scoreAggregators/SEOScoreAggregator";

/**
* Returns the text length assessment to use.
Expand Down Expand Up @@ -61,5 +62,7 @@ export default class TaxonomyAssessor extends Assessor {
new FunctionWordsInKeyphrase(),
new SingleH1Assessment(),
];

this._scoreAggregator = new SEOScoreAggregator();
}
}
Loading