Skip to content
This repository was archived by the owner on Sep 20, 2024. It is now read-only.

Publish report viewer: Report items sorting#6092

Merged
iLLiCiTiT merged 15 commits intodevelopfrom
enhancement/publisher-report-viewer-sorting
Jan 25, 2024
Merged

Publish report viewer: Report items sorting#6092
iLLiCiTiT merged 15 commits intodevelopfrom
enhancement/publisher-report-viewer-sorting

Conversation

@iLLiCiTiT
Copy link
Copy Markdown
Member

Changelog Description

Proposal of items sorting in Publish report viewer tool. Items are sorted by report creation time. Creation time is also added to publish report data when saved from publisher tool.

Additional info

For older report items is used file modification time as 'created_at'. Possible issues with this approach is that new dropped report may not be as last but in the middle, which may not be ideal. ISO timestamp should be probably used instead of "current timezone" timestamp.

Other possible approaches:

  1. Add 'stored_at' information when report file is dropped to the UI, and use the 'stored_at' for sorting instead.
  2. Completelly ignore value (column) sorting but use "custom sorting". User can manually move items around and int with sort value is stored for each item.

Testing notes:

  • New reports from publisher contain 'created_at'.
  1. Open Publish report viewer.
  2. Existing items should work as before.
  3. Drop new report item. Which should load just fine.
  4. UI should show 2 columns: 'Reports' and 'Created'. Second column is used for sorting by default.

@iLLiCiTiT iLLiCiTiT requested a review from BigRoy December 27, 2023 11:42
@ynbot ynbot added size/S Denotes a PR changes 100-499 lines, ignoring general files type: enhancement Enhancements to existing functionality labels Dec 27, 2023
@iLLiCiTiT iLLiCiTiT added community contribution and removed type: enhancement Enhancements to existing functionality size/S Denotes a PR changes 100-499 lines, ignoring general files labels Dec 27, 2023
@BigRoy
Copy link
Copy Markdown
Collaborator

BigRoy commented Dec 27, 2023

Out of curiosity - why wouldn't we just use the Create timestamp or Date modified timestamp of the file on disk?

Copy link
Copy Markdown
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

With this PR I'm unable to drop new error reports onto the left panel to load new reports if I remove an entry and try to drag 'n' drop it in directly again. I then need to first close the UI and restart it. (it migth be that this behavior is similar as before)

Also, if I drop in a file that's ALREADY in the UI then the UI appears to do nothing (no warning/error). I'd expect that if it should not add existing entries again, that instead it's best to highlight what matching entry already exists?

But maybe better is being able to load a file, again and again, as new entries?

@iLLiCiTiT
Copy link
Copy Markdown
Member Author

Out of curiosity - why wouldn't we just use the Create timestamp or Date modified timestamp of the file on disk?

I don't think those metadata are helpfull. UI will create and modify the file at the moment you drop the file in, or change label. Also create time of a file is usually not easy to pass to a different machine (especially to different OS).

With this PR I'm unable to drop new error reports onto the left panel to load new reports if I remove an entry and try to drag 'n' drop it in directly again. I then need to first close the UI and restart it. (it migth be that this behavior is similar as before)

That sounds like I've added a bug. Will look into it.

Also, if I drop in a file that's ALREADY in the UI then the UI appears to do nothing (no warning/error). I'd expect that if it should not add existing entries again, that instead it's best to highlight what matching entry already exists?

What if you'd drop 3 reports, 1 is new and 2 were already there, what should happen?

I can imagine that new entries are highlighted in some way, maybe? Probably not in this PR.

But maybe better is being able to load a file, again and again, as new entries?

That was actually a bug that you could do that. Each report has id, which is used for mapping in backend and also to store the report into file. Practically it is not possible to have 2 different reports with same id, until you manually modify them.

Copy link
Copy Markdown
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

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

Work as expected
image

@iLLiCiTiT iLLiCiTiT merged commit 77bfe7e into develop Jan 25, 2024
@iLLiCiTiT iLLiCiTiT deleted the enhancement/publisher-report-viewer-sorting branch January 25, 2024 09:35
@ynbot ynbot added this to the next-patch milestone Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants