Skip to content

Hotfix for NASA crater detection challenge results#7180

Merged
jmgasper merged 2 commits intomasterfrom
hotfix-review-summations
Feb 23, 2026
Merged

Hotfix for NASA crater detection challenge results#7180
jmgasper merged 2 commits intomasterfrom
hotfix-review-summations

Conversation

@jmgasper
Copy link
Collaborator

No description provided.

@jmgasper jmgasper requested a review from kkartunov as a code owner February 23, 2026 20:53
const tokenBecameAvailable = !previousToken && !!nextToken;

if (
checkIsMM(nextProps.challenge)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 maintainability]
The condition checkIsMM(nextProps.challenge) is used multiple times in this method. Consider extracting it into a variable for better readability and maintainability.

&& nextProps.statisticsData.length > 0;
const enteringMmDashboard = selectedTab !== DETAIL_TABS.MM_DASHBOARD
&& nextProps.selectedTab === DETAIL_TABS.MM_DASHBOARD;
const tokenBecameAvailable = !previousToken && !!nextToken;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The variable enteringMmDashboard is defined but not used in the subsequent logic. Ensure this condition is necessary or remove it to avoid confusion.

const hasProvisionalScore = !_.isNil(
normalizeScoreValue(submission.provisionalScore),
);
if (!hasFinalScore || hasProvisionalScore || _.isNil(latestProvisionalScore)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 readability]
The condition if (!hasFinalScore || hasProvisionalScore || _.isNil(latestProvisionalScore)) is complex and may lead to confusion. Consider breaking it down into smaller, named boolean variables to improve readability.

if (_.isNil(score)) {
return;
}
if (includeOnlyProvisional) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The check if (_.isNil(score)) { return; } is repeated in both buildMmSubmissionData and buildStatisticsData. Consider extracting this logic into a helper function to avoid duplication and improve maintainability.

return;
}
if (includeOnlyProvisional) {
const scoreType = getSummationScoreClassification(summation);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ maintainability]
The logic for determining whether to include only provisional scores is duplicated in both buildMmSubmissionData and buildStatisticsData. Consider refactoring this into a shared utility function to reduce redundancy.

@jmgasper jmgasper merged commit 487a868 into master Feb 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant