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

Highlight outdated feature entries on roadmap #4472

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 api/converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ def feature_entry_to_json_basic(fe: FeatureEntry,
},
'created': {'by': fe.creator_email, 'when': _date_to_str(fe.created)},
'updated': {'by': fe.updater_email, 'when': _date_to_str(fe.updated)},
'accurate_as_of': _date_to_str(fe.accurate_as_of),
'standards': {
'spec': fe.spec_link,
'maturity': {
Expand Down
2 changes: 2 additions & 0 deletions api/converters_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def test_feature_entry_to_json_basic__normal(self):
'by': '[email protected]',
'when': expected_date
},
'accurate_as_of': expected_date,
'standards': {
'spec': 'https://example.com/spec',
'maturity': {
Expand Down Expand Up @@ -206,6 +207,7 @@ def test_feature_entry_to_json_basic__feature_release(self):
'by': '[email protected]',
'when': expected_date
},
'accurate_as_of': expected_date,
'standards': {
'spec': 'https://example.com/spec',
'maturity': {
Expand Down
40 changes: 40 additions & 0 deletions client-src/elements/chromedash-roadmap-milestone-card.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class ChromedashRoadmapMilestoneCard extends LitElement {
showDates = false;
@property({type: Boolean})
signedIn = false;
@property({attribute: false})
stableMilestone!: number;

/**
* Returns the number of days between a and b.
Expand Down Expand Up @@ -234,6 +236,34 @@ class ChromedashRoadmapMilestoneCard extends LitElement {
`;
}

// A feature is outdated if it is scheduled to ship in the next 2 milestones,
// and its accurate_as_of date is at least 4 weeks ago.
_isFeatureOutdated(accurateAsOf) {
if (this.stableMilestone === 0) {
return false;
}
// If this feature is not shipping within two upcoming milestones, return false.
if (
!(
this.stableMilestone + 1 === this.channel?.version ||
this.stableMilestone + 2 === this.channel?.version
)
) {
return false;
}
if (!accurateAsOf) {
return true;
}
const accuateDate = Date.parse(accurateAsOf);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const accuateDate = Date.parse(accurateAsOf);
const accurateDate = Date.parse(accurateAsOf);

// 4-week period.
const gracePeriod = 4 * 7 * 24 * 60 * 60 * 1000;
if (accuateDate + gracePeriod < Date.now()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (accuateDate + gracePeriod < Date.now()) {
if (accurateDate + gracePeriod < Date.now()) {

return true;
}

return false;
}

_cardFeatureItemTemplate(f, shippingType) {
return html`
<li
Expand All @@ -249,6 +279,16 @@ class ChromedashRoadmapMilestoneCard extends LitElement {
${f.name}
</a>
<span class="icon_row">
${this._isFeatureOutdated(f.accurate_as_of)
? html`
<span
class="tooltip"
title="Feature outdated - last updated more than four weeks ago"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not quite "updated". Perhaps:

Suggested change
title="Feature outdated - last updated more than four weeks ago"
title="Feature outdated - last checked for overall accuracy more than four weeks ago"

>
<iron-icon icon="chromestatus:error" data-tooltip></iron-icon>
</span>
`
: nothing}
Comment on lines +282 to +291
Copy link
Collaborator

Choose a reason for hiding this comment

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

I note that you're missing a test for this behavior.

${ORIGIN_TRIAL.includes(shippingType)
? html`
<span class="tooltip" title="Origin Trial">
Expand Down
1 change: 1 addition & 0 deletions client-src/elements/chromedash-roadmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ export class ChromedashRoadmap extends LitElement {
.starredFeatures=${this.starredFeatures}
.highlightFeature=${this.highlightFeature}
?signedin=${this.signedIn}
.stableMilestone=${this.channels?.['stable']?.version}
@star-toggle-event=${this.handleStarToggle}
@highlight-feature-event=${this.handleHighlightEvent}
>
Expand Down
4 changes: 4 additions & 0 deletions client-src/elements/icons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,10 @@ const template = html`
d="M12 8c1.1 0 2-.9 2-2s-.9-2-2-2-2 .9-2 2 .9 2 2 2zm0 2c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2zm0 6c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2z"
></path>
</g>

<g id="error">
<path d="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-2h2v2zm0-4h-2V7h2v6z"/></path>
</g>
</defs>
</svg>
</iron-iconset-svg>
Expand Down