-
Notifications
You must be signed in to change notification settings - Fork 22
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
Removing the "Upcoming Closing Dates" Section in Dashboard #3211
Conversation
Not sure if I caught all the now-unused code in styling and script, I might have missed something? |
QA Summary
Test CoverageCoverage report for `packages/client`
Coverage report for `packages/server`
|
Terraform Summary
Hint: If "Terraform Format & Style" failed, run OutputValidation Output
Plan Summary
Pusher: @mayabose, Action: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mayabose This is a good start – visually, things are looking as I'd expect them to.
re your comment
Not sure if I caught all the now-unused code in styling and script, I might have missed something?
There are a few client-side items mentioned in #3155 that haven't been addressed yet:
- Delete the
packages/client/src/components/ClosingDatesTable.vue
component file and refactor thepackages/client
code to remove any/all remaining usages.- Delete the
packages/client/src/views/UpcomingClosingDatesView.vue
view file and refactor thepackages/client
code to remove any/all remaining usages, including its Vue route and any API calls.
These files need to be actually deleted (once a file is deleted locally, you can use git add
to stage the "change" as you would do with a modified file). Deleting the files locally and then running the client-side unit tests (yarn test:client
) should help uncover any other changes that might need to be made to the packages/server/
code.
Additionally, there are some API-side changes (also listed in #3155) that are still required before this PR can be approved:
API-side changes
- In
packages/server/src/routes/grants.js
, remove the/closestGrants/:perPage/:currentPage
API route handler.- In
packages/server/src/db/index.js
, remove thegetClosestGrants()
function, which should no remaining callers following removal of the/closestGrants/:perPage/:currentPage
route.- Check for and prune any/all other "dead code" that results from removing the
/closestGrants/:perPage/:currentPage
route (including unit tests, which may now be failing).
Let me know if anything in those remaining changes is unclear and we can chat further 🙂
Totally missed the second half of the original ticket - so sorry about that! However, I think I may still be missing some instances related to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great – approved!
Ticket #3155
Description
This ticket removes any code related to the upcoming dates section in the Dashboard, allowing the Recent Activity table to become larger.
b-col
container for the Upcoming Closing Dates is removedb-col cols="1"
container is removed to allow the Recent Activity container to shift leftScreenshots / Testing
Previous Dashboard:
New Dashboard:
Checklist