Skip to content

Update the collection page table with the latest dynamic data table options #1225

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Jul 9, 2025
Merged

Update the collection page table with the latest dynamic data table options #1225

merged 10 commits into from
Jul 9, 2025

Conversation

BenjaminCharmes
Copy link
Contributor

Closes #1192

@codecov Codecov
Copy link

codecov bot commented Jun 3, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.50%. Comparing base (2db7144) to head (dafc5ea).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1225      +/-   ##
==========================================
+ Coverage   74.42%   74.50%   +0.08%     
==========================================
  Files          66       66              
  Lines        4520     4535      +15     
==========================================
+ Hits         3364     3379      +15     
  Misses       1156     1156
Files with missing lines Coverage Δ
pydatalab/src/pydatalab/routes/v0_1/collections.py 73.46% <100.00%> (+3.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cypress Cypress
Copy link

cypress bot commented Jun 3, 2025 •
edited
Loading

datalab    Run #3573

Run Properties:  status check passed Passed #3573  •  git commit ad133eb8d0 ℹ️: Merge dafc5ea816a394b7cfbc7eae64a3606c3688b649 into 2db714447e8d60d771151ed0a6a0...
Project datalab
Branch Review bc/update-collection-table
Run status status check passed Passed #3573
Run duration 07m 45s
Commit git commit ad133eb8d0 ℹ️: Merge dafc5ea816a394b7cfbc7eae64a3606c3688b649 into 2db714447e8d60d771151ed0a6a0...
Committer Ben Charmes
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 504
View all changes introduced in this branch ↗︎

@BenjaminCharmes BenjaminCharmes marked this pull request as ready for review June 3, 2025 15:15
ml-evs
ml-evs reviewed Jun 11, 2025
View reviewed changes
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Thanks @BenjaminCharmes, apologies for the delay... I'd left this as a pending review comment.

@BenjaminCharmes BenjaminCharmes requested a review from ml-evs June 12, 2025 12:49
@ml-evs ml-evs force-pushed the bc/update-collection-table branch 2 times, most recently from 3f3eb71 to 7d14903 Compare June 16, 2025 11:43
@ml-evs ml-evs added webapp For issues/PRs pertaining to the web interface usability labels Jun 16, 2025
@ml-evs ml-evs force-pushed the bc/update-collection-table branch from 7d14903 to 6efe2fe Compare June 21, 2025 12:36
ml-evs
ml-evs reviewed Jun 21, 2025
View reviewed changes
Copy link
Member

@ml-evs ml-evs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

UI seems to work nicely, thanks @BenjaminCharmes! Just a couple of comments about the API endpoint and tests (and whether we should switch everything over to refcodes, hopefully not too annoying)

BenjaminCharmes and others added 3 commits June 23, 2025 17:01
@BenjaminCharmes @ml-evs
use refcodes rather than item ids
52b32c3 
Co-authored-by: Matthew Evans <git@ml-evs.science>
@BenjaminCharmes @ml-evs
Update CollectionInformation global filter
7538210 
Co-authored-by: Matthew Evans <git@ml-evs.science>
@BenjaminCharmes
add review correction and use refcode
38dc47d
@BenjaminCharmes BenjaminCharmes requested a review from ml-evs June 26, 2025 10:25
@ml-evs ml-evs requested a review from be-smith June 30, 2025 09:27
be-smith
be-smith previously approved these changes Jul 8, 2025
View reviewed changes
Copy link
Contributor

@be-smith be-smith left a comment

Choose a reason for hiding this comment

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

UI changes look great - and all the functionality seems to be working for me!

@ml-evs ml-evs force-pushed the bc/update-collection-table branch from 6b4d330 to dafc5ea Compare July 9, 2025 08:59
ml-evs
ml-evs approved these changes Jul 9, 2025
View reviewed changes
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Looks good and works well, thanks @BenjaminCharmes!

pydatalab/tests/server/test_samples.py

assert response.status_code == 400
data = response.get_json()
assert data["error"] == "No refcodes provided"
Copy link
Member

Choose a reason for hiding this comment

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

Just a comment that this made me realise the collections routes do not use standard errors (i.e., status = "error", then "message" = "whatever"). Not something to fix in this PR, but we should go through and unify all routes on this at some point.

@ml-evs ml-evs merged commit 436bea0 into main Jul 9, 2025
18 checks passed
@ml-evs ml-evs deleted the bc/update-collection-table branch July 9, 2025 14:16
be-smith pushed a commit that referenced this pull request Jul 30, 2025
@BenjaminCharmes @ml-evs @be-smith
Update the collection page table with the latest dynamic data table o…
f67e6b9 
…ptions (#1225)

Co-authored-by: Matthew Evans <git@ml-evs.science>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ml-evs ml-evs ml-evs approved these changes

@be-smith be-smith be-smith left review comments

@jdbocarsly jdbocarsly Awaiting requested review from jdbocarsly jdbocarsly is a code owner

Assignees
No one assigned
Labels
usability webapp For issues/PRs pertaining to the web interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update the collection page table with the latest dynamic data table options
3 participants
@BenjaminCharmes @ml-evs @be-smith