-
Notifications
You must be signed in to change notification settings - Fork 33
30749 UI: Audit history component #2285
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
Conversation
|
/gcbrun |
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.
Pull request overview
This pull request implements an Audit History component and removes the read-only mode from the Review Decision component, streamlining the analyst queue workflow for transfer reviews.
Changes:
- Removed read-only display mode from ReviewDecision component, keeping only the editable form
- Added AuditHistory component to display historical review steps in a collapsible format
- Changed ReviewDecision visibility condition from showing for IN_REVIEW/APPROVED/DECLINED to only IN_REVIEW status
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ppr-ui/src/store/analystQueue.ts | Removed shouldShowReviewDecision computed property, replaced with existing isInReview for simpler state management |
| ppr-ui/src/pages/mhrInformation/MhrQueueTransfer/index.vue | Added AuditHistory component and updated ReviewDecision visibility to show only during IN_REVIEW status |
| ppr-ui/src/composables/analystQueue/resources.ts | Updated import path for enumToLabel to use specific path instead of barrel export |
| ppr-ui/src/components/queue/ReviewDecision.vue | Removed read-only mode logic, simplified to always show editable form with approve/decline buttons |
| ppr-ui/src/components/queue/AuditHistory.vue | New component displaying review history in a collapsible format with date/time, status, and notes |
| ppr-ui/package.json | Version bump to 6.0.5 and added Vue compiler packages (@vue/compiler-dom, @vue/server-renderer) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/gcbrun |
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
copilot review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ppr-ui/package.json
Outdated
| "@vue/compiler-dom": "3.5.20", | ||
| "@vue/compiler-sfc": "3.5.20", | ||
| "@vue/server-renderer": "3.5.20", |
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.
Could you give some details on why the added packages here?
is compiler-dom used?
also vue/server-renderer is for vue 2 server side rendering support.
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.
I got renderer errors when running the unit tests locally. The packages @vue/compiler-dom (3.5.20) and @vue/server-renderer (3.5.20) were added specifically for unit testing. @vue/compiler-sfc (3.5.20) was already an existing dependency; I only updated it from 3.5.2 to 3.5.20 based on Copilot’s suggestion.
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.
Hmm.. and your sure these packages are preventing those errors?
If these packages were installed, i would expect changes to the lockfile.
We have a ticket to look to knowledge transfer on the testing.
Are you ok with removing these packages from this PR and we can look at this together?
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.
no problem. the changes in packages has been rolled back
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.
Thanks!
doug-lovett
left a comment
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 good, nothing to to add to Cameron's comments.
|
Looks good, just some minor lint issues! Is this ready for review or still in progress? it's currently set as a draft pr. |
|
/gcbrun |
It looks like the lint error is related to updating pnpm-lock.yaml. Do you have suggestions on how to fix it? |
It is because there is changes to the package.json file: If you removed the un-installed package from the package.json file, it should be ok. |
cameron-eyds
left a comment
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.
Thanks Eve, looks great!
Issue #: /bcgov/entity###
bcgov/entity#30749
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the PPR license (Apache 2.0).