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

Add last scan date below upsell button #4407

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Add last scan date below upsell button #4407

merged 3 commits into from
Apr 29, 2024

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Apr 10, 2024

References:

Jira: MNTOR-2701
Figma: https://www.figma.com/file/DTbmXzCQCw2PxXpHQW8yfW/Monitor-MVP-Enhancements?type=design&node-id=24-538&mode=design

Description

Lets a user know when their last scan took place.

Note that I also moved UpsellBadge.tsx into the toolbar folder, since it's shown in the toolbar.

Screenshot (if applicable)

image

image

How to test

Open the dashboard with a user who has performed a scan.

Checklist (Definition of Done)

  • Localization strings (if needed) have been added.
  • Commits in this PR are minimal and have descriptive commit messages.
  • I've added or updated the relevant sections in readme and/or code comments
  • I've added a unit test to test for potential regressions of this bug. - No real business logic added.
  • Product Owner accepted the User Story (demo of functionality completed) or waived the privilege.
  • All acceptance criteria are met.
  • Jira ticket has been updated (if needed) to match changes made during the development process.
  • Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

@Vinnl Vinnl added the Review: XS Code review time: up to 30min label Apr 10, 2024
@Vinnl Vinnl requested review from codemist and flozia April 10, 2024 14:02
@Vinnl Vinnl self-assigned this Apr 10, 2024
@Vinnl Vinnl requested a review from flodolo as a code owner April 10, 2024 14:02
@Vinnl
Copy link
Collaborator Author

Vinnl commented Apr 10, 2024

Putting back in draft mode - it needs to be put behind a feature flag.

@Vinnl Vinnl marked this pull request as draft April 10, 2024 16:50
}

const scan = await knex("onerep_scans")
.first()
.where("onerep_profile_id", onerepProfileId)
.orderBy("created_at", "desc");

return scan ?? null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting! What does this do?
I thought the nullish coalescing would only return the right-hand side if the left-hand side was already null.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nullish coalescing returns the right-hand side if the left-hand side is null or undefined. So this ensures we return null if there was no result.

@Vinnl
Copy link
Collaborator Author

Vinnl commented Apr 11, 2024

Flag added in dd3d4d8.

@Vinnl Vinnl marked this pull request as ready for review April 11, 2024 11:12
Copy link
Collaborator

@rhelmer rhelmer left a comment

Choose a reason for hiding this comment

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

@Vinnl would you mind adding an entry for this into the nimbus.yaml? I think it makes sense to do as part of landing a feature, just as we do with the feature flag name.

@Vinnl
Copy link
Collaborator Author

Vinnl commented Apr 25, 2024

@rhelmer I was going to do that as part of setting up the experiment in the first place, in a separate PR, since that has its own ticket (and is probably a fair bit of (review) work by itself).

Edit: #4443

Copy link
Collaborator

@codemist codemist left a comment

Choose a reason for hiding this comment

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

LGTM but not sure if we're still rolling this out since I think we're looking to redesign it?

@rhelmer rhelmer removed their request for review April 25, 2024 19:18
After all, it's part of the toolbar.
(Only shown if the user has performed a scan.)
Supposedly that's a prerequisite for making it part of an
experiment.
@Vinnl
Copy link
Collaborator Author

Vinnl commented Apr 29, 2024

LGTM but not sure if we're still rolling this out since I think we're looking to redesign it?

I'm not actually sure if we made a decision on that, but we're going to run an experiment for it, so we'll first have to set that up anyway. Possibly we'll move it over a bit after that. (Which isn't a major redesign anyway, just putting it to the left of the badge.)

@Vinnl Vinnl merged commit cf28989 into main Apr 29, 2024
14 checks passed
@Vinnl Vinnl deleted the MNTOR-2701-last-scan branch April 29, 2024 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants