Skip to content

LG-11738 Update sp user count report#9719

Merged
Sgtpluck merged 3 commits intomainfrom
dmm/LG-11738/fix-sp-user-count-report
Dec 7, 2023
Merged

LG-11738 Update sp user count report#9719
Sgtpluck merged 3 commits intomainfrom
dmm/LG-11738/fix-sp-user-count-report

Conversation

@Sgtpluck
Copy link
Contributor

@Sgtpluck Sgtpluck commented Dec 6, 2023

🎫 Ticket

https://cm-jira.usa.gov/browse/LG-11738

🛠 Summary of changes

Updates sp user count report to use the table report format

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before: screenshot of an email with the user count data listed out without any formatting or details
After: screenshot of an email with the user count data listed in a beautiful table, along with an overview table that has the day and the issuer

Copy link
Contributor

@nprimak nprimak left a comment

Choose a reason for hiding this comment

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

That does look much better, nice!

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

Choose a reason for hiding this comment

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

you can do Time.zone.today to get around this rubocop

Suggested change
['Report Generated', Date.today.to_s], # rubocop:disable Rails/Date
['Report Generated', Time.zone.today.to_s],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sure! i was just keeping it consistent with what you'd used in the idv report, as i figured there had probably been a reason for it! happy to just update it to Time.zone.today -- i will also update the other reports to make it consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

LOL I think that was because those reports were loaded from the command line and didn't have Rails set, so Time.zone was nil. Inside this background job will always be Rails so it's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, whoops

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Suggested change
let(:date) { Date.today.to_s } # rubocop:disable Rails/Date
let(:date) { Time.zone.today.to_s }

@Sgtpluck Sgtpluck force-pushed the dmm/LG-11738/fix-sp-user-count-report branch from 012d8c1 to 7eb3a7c Compare December 7, 2023 13:01
@Sgtpluck Sgtpluck force-pushed the dmm/LG-11738/fix-sp-user-count-report branch from 7eb3a7c to 6fe997f Compare December 7, 2023 13:30
@Sgtpluck Sgtpluck force-pushed the dmm/LG-11738/fix-sp-user-count-report branch from 6fe997f to d261cb7 Compare December 7, 2023 14:01
@Sgtpluck Sgtpluck merged commit 3832a61 into main Dec 7, 2023
@Sgtpluck Sgtpluck deleted the dmm/LG-11738/fix-sp-user-count-report branch December 7, 2023 15:56
@jmhooper jmhooper mentioned this pull request Dec 12, 2023
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.

3 participants