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

Rewrite audit report trail #791

Merged
merged 15 commits into from
Mar 22, 2024

Conversation

Lovelyfin00
Copy link
Contributor

@Lovelyfin00 Lovelyfin00 commented Mar 11, 2024

Fixes #761

Fix Summary

  • Migrated the Audit Trail report from JSP to new React UI
  • Added breadcrumbs

Screen recording

12.03.2024_11.38.44_REC.mp4

@Lovelyfin00 Lovelyfin00 marked this pull request as ready for review March 12, 2024 10:42
@Lovelyfin00
Copy link
Contributor Author

Hi @mozzy11 @manishjha-04 @sherrif10 @CalebSLane

This PR is ready for review now
Thanks a bunch😅😅

Copy link
Collaborator

@mozzy11 mozzy11 left a comment

Choose a reason for hiding this comment

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

hello @Lovelyfin00 , Nice progress.
Tis will require rewriting the actual report page in React

@Lovelyfin00
Copy link
Contributor Author

Lovelyfin00 commented Mar 18, 2024

hello @Lovelyfin00 , Nice progress. Tis will require rewriting the actual report page in React

Hi @mozzy11

Yeah I did that. You can checkout the video I included in the description. When it comes to making the request, I called the endpoint but got an html response and not a json. So there was no way for me to display it which is why i had to redirect to the url

@mozzy11
Copy link
Collaborator

mozzy11 commented Mar 18, 2024

yeah ,
youll need to re-write this controller as a REST controller

Copy link

👋 Hi, @Lovelyfin00,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@github-actions github-actions bot added the merge conflict Merge Conflicts label Mar 18, 2024
@Lovelyfin00
Copy link
Contributor Author

yeah , youll need to re-write this controller as a REST controller

That makes alot of sense, thanks. Is there a way to get a correct accessionNumber @mozzy11

@mozzy11
Copy link
Collaborator

mozzy11 commented Mar 18, 2024

@Lovelyfin00 what do you mean to get the correct correct accessionNumber ??

@Lovelyfin00
Copy link
Contributor Author

Lovelyfin00 commented Mar 18, 2024

@Lovelyfin00 what do you mean to get the correct correct accessionNumber ??

I actually meant Lab No @mozzy11
When i search using Lab No, I don't know how the UI displays the result because I don't have a correct lab No to search for

@github-actions github-actions bot removed the merge conflict Merge Conflicts label Mar 20, 2024
@Lovelyfin00
Copy link
Contributor Author

Hi @mozzy11
I don't know Java so I might not be able to writeout the code for the rest controller

frontend/src/App.js Outdated Show resolved Hide resolved
@mozzy11
Copy link
Collaborator

mozzy11 commented Mar 21, 2024

Hello @Lovelyfin00 , addres the above comments , i can merge this in and Have some one who can complete the API

@Lovelyfin00
Copy link
Contributor Author

Hi @mozzy11
I have made the requested changes. Here is a video demo of them

refactor.mp4

I keep seeing this error even though I have added the id to both translation files.


image

@github-actions github-actions bot added the merge conflict Merge Conflicts label Mar 22, 2024
Copy link

👋 Hi, @Lovelyfin00,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@mozzy11 mozzy11 merged commit dd551cd into I-TECH-UW:develop_3x Mar 22, 2024
4 of 5 checks passed
@@ -20,21 +21,18 @@
"from.title": "De",
"to.title": "À",
"report.label.site.dateType": "Type de date",
"banner.menu.reports": "Rapports",
Copy link
Contributor

@parthnagdev parthnagdev Mar 25, 2024

Choose a reason for hiding this comment

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

hey @Lovelyfin00 why was this line "banner.menu.reports": "Rapports", removed? what issue was it causing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parthnagdev
Sorry, I just cross checked
I thought it was duplicated as a result of merge conflicts.
I can add it back right now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened a PR for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflict Merge Conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re - Write AuditTrailReport In the new React UI
5 participants