Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ workflows:
branches:
only:
- develop
- PM-3686_group-submissions-in-challenge-details

- "build-prod":
context: org-global
Expand Down
44 changes: 25 additions & 19 deletions src/shared/components/Dashboard/Challenges/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,32 @@ export default function ChallengesFeed({
<LoadingIndicator />
</div>
) : (
(challenges || []).map(challenge => (
<div styleName="row" key={challenge.id}>
<a
href={`/challenges/${challenge.id}`}
target="_blank"
rel="noreferrer"
>
{challenge.name}
</a>
<div styleName="prize">
<span styleName="amount">
{`$${_.sum(
challenge.prizeSets
.filter(set => set.type === 'PLACEMENT')
.map(item => _.sum(item.prizes.map(prize => prize.value))),
).toLocaleString()}`}
</span>
(challenges || []).map((challenge) => {
const placementPrizes = challenge.prizeSets
.filter(set => set.type === 'PLACEMENT')
.flatMap(item => item.prizes);
const prizeTotal = _.sum(placementPrizes.map(prize => prize.value));
const prizeType = placementPrizes.length > 0 ? placementPrizes[0].type : null;

Choose a reason for hiding this comment

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

[⚠️ correctness]
The prizeType is determined by the first element of placementPrizes. If placementPrizes contains mixed types, this logic might not accurately reflect the prize type. Consider verifying that all prizes are of the same type or adjusting the logic to handle mixed types appropriately.

const isPointBasedPrize = prizeType === 'POINT';
const prizeSymbol = isPointBasedPrize ? '' : '$';

return (
<div styleName="row" key={challenge.id}>
<a
href={`/challenges/${challenge.id}`}
target="_blank"
rel="noreferrer"
>
{challenge.name}
</a>
<div styleName="prize">
<span styleName="amount">
{`${prizeSymbol}${prizeTotal.toLocaleString()}`}
</span>
</div>
</div>
</div>
))
);
})
)}
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import React from 'react';
import PT from 'prop-types';
import MarkdownRenderer from 'components/MarkdownRenderer';

import './styles.scss';

Expand All @@ -19,13 +20,10 @@ const ChallengeSpecTab = ({ challenge }) => (
Challenge Overview
</h2>
<div
/* eslint-disable react/no-danger */
dangerouslySetInnerHTML={{
__html: challenge.description,
}}
/* eslint-enable react/no-danger */
styleName="rawHtml"
/>
>
<MarkdownRenderer markdown={challenge.description} />

Choose a reason for hiding this comment

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

[❗❗ security]
Replacing dangerouslySetInnerHTML with MarkdownRenderer is a positive change for security, as it reduces the risk of XSS vulnerabilities. However, ensure that MarkdownRenderer itself is secure and properly sanitizes the input to prevent any potential security issues.

</div>
</article>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default function Submission(props) {
} = props;
const formatDate = date => moment(+new Date(date)).format('MMM DD, YYYY hh:mm A');
const onDownloadSubmission = onDownload.bind(1, submissionObject.id);
const safeForDownloadCheck = safeForDownload(submissionObject.url);
const safeForDownloadCheck = safeForDownload(submissionObject);

Choose a reason for hiding this comment

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

[❗❗ correctness]
The safeForDownload function now takes submissionObject instead of submissionObject.url. Ensure that safeForDownload is designed to handle the entire object and not just the URL, as this change might affect its behavior.

const onDownloadArtifacts = onOpenDownloadArtifactsModal.bind(1, submissionObject.id);
const onOpenRatingsList = onOpenRatingsListModal.bind(1, submissionObject.id);
const onOpenReviewApp = () => {
Expand Down
2 changes: 2 additions & 0 deletions src/shared/components/challenge-detail/Header/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@

.block-tags-container {
display: flex;
flex-wrap: wrap;
gap: 8px;
}

Expand All @@ -559,6 +560,7 @@
background-color: $color-black-10 !important;
border: none;
margin: 0;
padding: 4px 8px;

&:hover {
background-color: #d4d4d4 !important;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export default function SubmissionHistoryRow({
finalScore,
provisionalScore,
submissionTime,
createdAt,
isReviewPhaseComplete,
status,
challengeStatus,
Expand All @@ -42,9 +43,11 @@ export default function SubmissionHistoryRow({
};
const provisionalScoreValue = parseScore(provisionalScore);
const finalScoreValue = parseScore(finalScore);
const submissionMoment = submissionTime ? moment(submissionTime) : null;

const timeField = isMM ? submissionTime : createdAt;

Choose a reason for hiding this comment

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

[⚠️ correctness]
The logic for selecting timeField based on isMM could lead to unexpected behavior if both submissionTime and createdAt are null. Consider adding a fallback or handling this case explicitly to avoid potential issues.

const submissionMoment = timeField ? moment(timeField) : null;
const submissionTimeDisplay = submissionMoment
? `${submissionMoment.format('DD MMM YYYY')} ${submissionMoment.format('HH:mm:ss')}`
? submissionMoment.format('MMM DD, YYYY HH:mm')
: 'N/A';
const getInitialReviewResult = () => {
if (status === 'failed') return <FailedSubmissionTooltip />;
Expand Down Expand Up @@ -85,13 +88,17 @@ export default function SubmissionHistoryRow({
{getFinalScore()}
</div>
</div>
<div styleName="col-3 col">
<div styleName="mobile-header">PROVISIONAL SCORE</div>
<div>
{getInitialReviewResult()}
</div>
</div>
<div styleName={`col-4 col ${isMM ? 'mm' : ''}`}>
{
isMM && (
<div styleName="col-3 col">
<div styleName="mobile-header">PROVISIONAL SCORE</div>
<div>
{getInitialReviewResult()}
</div>
</div>
)
}
<div styleName={`${isMM ? 'col-4' : 'col-3'} col ${isMM ? 'mm' : ''}`}>

Choose a reason for hiding this comment

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

[💡 readability]
The conditional logic for setting the class name of the div is a bit complex and could be simplified for better readability. Consider extracting the class name logic into a separate variable to improve clarity.

<div styleName="mobile-header">TIME</div>
<div>
{submissionTimeDisplay}
Expand Down Expand Up @@ -134,6 +141,8 @@ SubmissionHistoryRow.defaultProps = {
provisionalScore: null,
isReviewPhaseComplete: false,
isLoggedIn: false,
createdAt: null,
submissionTime: null,
};

SubmissionHistoryRow.propTypes = {
Expand All @@ -154,7 +163,11 @@ SubmissionHistoryRow.propTypes = {
submissionTime: PT.oneOfType([
PT.string,
PT.oneOf([null]),
]).isRequired,
]),
createdAt: PT.oneOfType([
PT.string,
PT.oneOf([null]),
]),
challengeStatus: PT.string.isRequired,
isReviewPhaseComplete: PT.bool,
auth: PT.shape().isRequired,
Expand Down
Loading
Loading