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

[SVCS-146] In GDrive, a .gdoc and a .docx with the same name don't fully register #229

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented May 30, 2017

Purpose

When a gFile, (.gdoc, .gslides etc) gets exported we change the ext to .docx, .ppt etc. If this happens one file at a time waterbulter will deal with conflicts appropriately and prompt the user to rename/replace the file, but when multiple files are exported and uploaded coherently (like during registration or a folder drop, WB will overlook the conflict and replace/overwrite files with export conflicts.

For example: two files named test.gdoc and test.docx will cause a naming conflict when exported because they both will be exported as a docx file called test.docx.

This fix will modify the export path to append to the new extension. so test.gdoc and test.docx will be test.gdoc.docx and test.docx .

Changes

Simple change to metadata path property and modification so tests fit the new behavior.

Side Effects

Exported paths retain their old extensions, new extensions are appended.

Ticket

https://openscience.atlassian.net/browse/SVCS-146

…doc and test.docx will cause a naming conflict when exported to a new provider, like during registration) and change tests to accommodate.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.235% when pulling ff09371 on Johnetordoff:register-gdrive-export-names into 753bdd2 on CenterForOpenScience:develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.235% when pulling 64a1aa9 on Johnetordoff:register-gdrive-export-names into 753bdd2 on CenterForOpenScience:develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.235% when pulling 64a1aa9 on Johnetordoff:register-gdrive-export-names into 753bdd2 on CenterForOpenScience:develop.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.68% when pulling 4b105e3 on Johnetordoff:register-gdrive-export-names into b787a92 on CenterForOpenScience:develop.

…erbutler into register-gdrive-export-names

* 'develop' of https://github.com/CenterForOpenScience/waterbutler: (90 commits)
  Remove impossible conditional from _delete_folder
  Remove unnecessary check for sha.
  Remove tests for the never called is_sha function.
  Remove is_sha and _is_sha functions which are never called.
  Remove isinstance check that data is not list.
  Remove this check which is also made in function _fetch_tree
  Handle all github revision-identification parameters
  bump version & update changelog
  Allow NoneType checksums for non-exportable files.
  Add PR template.
  Fix s3 metadata_headers path tests
  Document APIv1 query parameters
  Remove strange use of query params
  Remove unused path property from BoxRevision
  Prune unused methods and error-handling from Box
  Removed three unused googledrive methods
  Rewrite osfstorage tests, add fixtures and metadata tests
  Increasing test coverage for figshare provider, path, and metadata
  bump version & update changelog
  Expand test coverage for owncloud provider+metadata
  ...

# Conflicts:
#	tests/providers/googledrive/test_metadata.py
…erbutler into register-gdrive-export-names

* 'develop' of https://github.com/CenterForOpenScience/waterbutler:
  Revert "Handle all github revision-identification parameters"
  Add __init__.py to modules missing them
  Adding AuthType over `is_source`
  Removing old comment
  Test unsupported actions in auth handler
  explicit auth provider source/destination w/ custom v1 post actions
  Allow copy requests to public resources
@coveralls
Copy link

coveralls commented Nov 17, 2017

Coverage Status

Coverage increased (+0.8%) to 89.942% when pulling d5eba8c on Johnetordoff:register-gdrive-export-names into ba55731 on CenterForOpenScience:develop.

Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

For ⛺️ 🔥 :

This PR looks good to me. Issue is in the ticket is clear and the fix is simple and elegant. I suggest this to be included in the next release.

For CR:

The commit history looks bad. I will cherry pick your commits and then rebase on the latest develop. I will approve it after I tested locally 🎆 .

@cslzchen cslzchen self-assigned this Jun 27, 2018
@cslzchen cslzchen removed their assignment Jul 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants