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

6574 filenames #6893

Merged
merged 8 commits into from
May 7, 2020
Merged

6574 filenames #6893

merged 8 commits into from
May 7, 2020

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented May 4, 2020

What this PR does / why we need it:

Several bugs were found while investigating filenames:

  • From the "edit metadata for files" page, you could give files the same name.
  • Via API, it was possible to give rename files to have the same name.
  • Via API, it was possible to "move" files to directory with a file that has the same name, creating a duplicate.

Which issue(s) this PR closes:

Closes #6574

Special notes for your reviewer:

Methods are in IngestUtil because that's where similar methods are found. Previous methods were focused on returning a unique filename such as README-1.md. The new methods are focused on identifying duplicates.

Suggestions on how to test this:

Try exercising the bugs above on develop vs. this branch. The API to use is this one, passing "label" and "directoryLabel" in the JSON: http://guides.dataverse.org/en/4.20/api/native-api.html#updating-file-metadata

Does this PR introduce a user interface change?:

Yes, if you try to rename a file to create a naming conflict, you will see an error like this:

Screen Shot 2020-05-01 at 5 10 27 PM

Is there a release notes update needed for this change?:

I don't believe so.

Additional documentation:

None.

@sekmiller sekmiller self-assigned this May 4, 2020
@coveralls
Copy link

coveralls commented May 4, 2020

Coverage Status

Coverage decreased (-0.01%) to 19.614% when pulling e79280b on 6574-filenames into 5285381 on develop.

Copy link
Contributor

@sekmiller sekmiller left a comment

Choose a reason for hiding this comment

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

suggested some code consolidation. overall looks good.

*/
public static boolean conflictsWithExistingFilenames(String label, String directoryLabel, List<FileMetadata> fileMetadatas, DataFile dataFile) {
List<String> filePathsAndNames = new ArrayList<>();
for (FileMetadata fileMetadata : fileMetadatas) {
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to combine this with findDuplicateFilenames so that the UI and api are using the same method? You're passing in the filemetadata list from the dataset version which is the param for findDuplicateFilenames

Copy link
Member Author

Choose a reason for hiding this comment

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

@sekmiller good idea. Fixed in 8d6a5c8.

@sekmiller sekmiller assigned pdurbin and unassigned sekmiller May 6, 2020
@pdurbin pdurbin removed their assignment May 6, 2020
@pdurbin
Copy link
Member Author

pdurbin commented May 6, 2020

I addressed code review in 8d6a5c8 (reduced duplicate code). I also added the new test class to the API test suite in dc77ac1 and merged the latest from develop.

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.

File Upload - Allow files of the same name within different directories of a dataset
5 participants