-
Notifications
You must be signed in to change notification settings - Fork 0
Manage file added again to same session, data is updated. #21
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
Open
gbastien
wants to merge
23
commits into
main
Choose a base branch
from
PARAF-359_file_added_to_same_session_again
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
df35686
Manage file added again to same session, data is updated.
gbastien caf6fcd
Completed test_add_files_already_exist_is_updated to show that filena…
gbastien 66d8df8
Check added to same session
gbastien 795cc16
Completed test_add_files_already_exist_is_updated regarding session s…
gbastien 19e094f
doc
gbastien e425dcf
Manage size
gbastien 9eefa2e
Added event to make sure session data (size, title, filename) is upda…
gbastien f9cef7c
Merge remote-tracking branch 'origin/main' into PARAF-359_file_added_…
gbastien 343e978
Session size is now always correct as it is update on add/edit/remove…
gbastien a0ff5e9
Added collective.iconifiedcategory to auto-checkout
gbastien ba0f644
Setup coverage
gbastien 7aef366
Removed double imio.annex
gbastien 70620ab
Removed double get_sessions_for
gbastien 84bd591
Install correct requirements-4.3.txt
gbastien b0d39a6
Run correct cfg file
gbastien ee3b6c4
Coverage gha
gbastien 006d4cb
Try to fix coveralls
gbastien fa8e300
Install liblma-dev but not wheel that was already installed by requir…
gbastien 820ea31
Pinned coverage = 5.3.1
gbastien c753000
Try to force pip3.8
gbastien 01581a4
Py3
gbastien 5e60ef1
Removed ls command
gbastien 385efb9
Show that without adding it again, data is updated during the modifie…
gbastien File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce qu'il ne faudrait pas tester ici que le filename et le title sont déjà mis à jour dans la session même avant de l'ajouter à nouveau ? Si oui, alors on n'est plus vraiment dans le contexte de test_utils mais plutôt test_events
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je vais compléter test_categorized_element_updated_event avec changement file, çà teste que c'est bien màj lors d'un edit, et donc ici c'est la création, on a le postulat que dans ce cas c'est bien à jour, d'ailleurs on le voit dans ce test, sinon on re-teste des choses déjà testées... Il me semble...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
à voir car création ou ajout même annexe ne passe pas par l'event de modif, c'est juste le add_files_to_session qui gère cela, çà va mettre à jour les infos correctement, le code lié à l'event est ci-dessous dans le "edit file"...
Je vais push un test_categorized_element_updated_event + complet où on teste aussi que les infos filesize sont mises à jour, mais c'est double test pour moi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chris-adam en effet le test a évolué petit à petit, et la partie "add" après le "modify" a été enlevée pour montrer que c'est màj sans re-add, j'avais lu trop vite car ton commentaire était "trop haut" dans le test, ici çà me semble bon comme çà on pourrait merger à mon sens