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

Improve Commit viewer #2712

Merged
merged 4 commits into from
Apr 18, 2024
Merged

Improve Commit viewer #2712

merged 4 commits into from
Apr 18, 2024

Conversation

soininen
Copy link
Contributor

@soininen soininen commented Apr 18, 2024

Commit viewer can now handle large databases as well.

  • The Affected items tree has been replaced by tabs, each tab corresponding to different item type and containing a table of affected items.
  • Affected items are now fetched in a worker thread to keep the Viewer responsive.
  • We now fetch maximum of 400-500 affected items and show the number of unfetched items in a text label. It does not seem to make sense to load thousands of items into the table.
  • Other small usability improvements.
  • The item_type and id_ parameters of DatabaseManager.get_value() have been replaced by item parameter which makes the method faster when the item is already at hand (the benchmark runs 1.9x faster).

Fixes #2602

Checklist before merging

  • Documentation is up-to-date
  • Release notes have been updated
  • Unit tests have been added/updated accordingly
  • Code has been formatted by black
  • Unit tests pass

- DB worker's commit cache uses DB ids, not TempIds
- We shouldn't fetch all when getting items for commit, it doesn't
  update commit cache nor make sense for huge databases.

Re #2602
The message hopefully helps users to understand why the viewer
sometimes shows no affected items.

Re #2602
- Affected items are now listed in tables and the tables are tabbed
  by item type. The previous tree view could not really handle
  large number of items usability or performance wise.
- Affected items are now fetched in a worker thread to keep the
  window responsive.
- Parameter values are now shown like in the Parameter value table
  in DB editor proper instead of showing the JSON representation.
  This seems to give a huge performance boost: apparently Qt is not
  great when it needs to render large amounts of text in a tiny
  table cell.

Re #26062
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

Attention: Patch coverage is 81.78694% with 53 lines in your changes are missing coverage. Please review.

Project coverage is 65.70%. Comparing base (17da6a2) to head (eec1d8c).
Report is 1 commits behind head on 0.8-dev.

Files Patch % Lines
...netoolbox/spine_db_editor/widgets/commit_viewer.py 68.78% 41 Missing and 8 partials ⚠️
...e_db_editor/ui/commit_viewer_affected_item_info.py 95.45% 0 Missing and 1 partial ⚠️
...pinetoolbox/spine_db_editor/ui/db_commit_viewer.py 98.91% 0 Missing and 1 partial ⚠️
...inetoolbox/spine_db_editor/widgets/custom_menus.py 0.00% 1 Missing ⚠️
spinetoolbox/spine_db_manager.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           0.8-dev    #2712      +/-   ##
===========================================
+ Coverage    65.52%   65.70%   +0.17%     
===========================================
  Files          194      196       +2     
  Lines        35674    35866     +192     
  Branches      6066     6090      +24     
===========================================
+ Hits         23376    23565     +189     
+ Misses       11049    11042       -7     
- Partials      1249     1259      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@soininen soininen merged commit 516898b into 0.8-dev Apr 18, 2024
@soininen soininen deleted the 2602_fix_commit_viewer branch April 18, 2024 11:25
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.

1 participant