Do not update 'files' reference when saving item #1055
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1055 +/- ##
=======================================
Coverage 70.90% 70.90%
=======================================
Files 64 64
Lines 4276 4276
=======================================
Hits 3032 3032
Misses 1244 1244
🚀 New features to boost your workflow:
|
datalab
|
||||||||||||||||||||||||||||
| Project |
datalab
|
| Branch Review |
ml-evs/fix-files
|
| Run status |
|
| Run duration | 08m 30s |
| Commit |
|
| Committer | Matthew Evans |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
504
|
| View all changes introduced in this branch ↗︎ | |
9218c19 to
7f7f8bb
Compare
|
Actually, converting this to a draft to add proper tests and a more general solution at a later date. |
7f7f8bb to
a90f53d
Compare
BenjaminCharmes
pushed a commit
that referenced
this pull request
May 16, 2025
BenjaminCharmes
added a commit
that referenced
this pull request
May 16, 2025
author Benjamin CHARMES <benjamin.charmes@gmail.com> 1747149404 +0200 committer Benjamin CHARMES <benjamin.charmes@gmail.com> 1747395376 +0200 Upgrade Bootstrap 5, better mobile optimisation, refactor e2e tests and tidy up! Add two more components Fix cypress test with new classes Open Modal faster to try to fix Electron cypress test Add the old Modal Re-add new Modal to check which tests fails Update e2e test batchSampleFeature to check that Modal is open before typing Update cypress commands to close Modal Update cypress commands to close Modal Fix uppercase typo Fix uppercase typo Fix getSubmitButton Fix cypress e2e Fix cypress e2e Final cypress fix Improving column management in Datatable (#1170) Co-authored-by: Matthew Evans <git@ml-evs.science> Resolve conflicts Resolve conflicts Refactor e2e tests Fix typo in sampleTablePage Fix typo in sampleTablePage Do not update 'files' reference when saving item (#1055) Improving column management in Datatable (#1170) Co-authored-by: Matthew Evans <git@ml-evs.science> Resolve conflicts Remove 'boostrap' Fix search bar height
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I accidentally depended on an item having
filessaved in the db, where actually it should only be a field constructed for the API response. This crept through as callingsave-itemfrom the app included this field, and as it fits the pydantic model, it was being saved in the database.We still need a better way of distinguishing such fields between the database and API, by adding special validators to the base entry types and annotating them as such in our models, but this just patches this problem for now.