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

Introduce UPSERT_RESOURCE to files mutations #5130

Merged
merged 3 commits into from
Jun 14, 2021

Conversation

pascalwengerter
Copy link
Contributor

Description

Merged PUSH_NEW_RESOURCE and UPDATE_RESOURCE to UPSERT_RESOURCE that checks if a file already exists and then either inserts or updates the resource

Related Issue

@pascalwengerter pascalwengerter requested a review from kulmann May 20, 2021 13:27
@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsersRoot2 failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15561/

webUISharingInternalUsersToRootPreviews-shareWithUsers-feature-54.png

webUISharingInternalUsersToRootPreviews-shareWithUsers-feature-54.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUISharingInternalUsersRoot1 failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15561/

webUISharingInternalUsersToRoot-shareWithUsers-feature-260.png

webUISharingInternalUsersToRoot-shareWithUsers-feature-260.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingPublicBasic failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15561/

webUISharingPublicBasic-publicLinkPublicActions-feature-12.png

webUISharingPublicBasic-publicLinkPublicActions-feature-12.png

@pascalwengerter pascalwengerter force-pushed the 20052021_correctly-update-resource-in-filestable branch 2 times, most recently from b7f522a to 9c525bb Compare May 20, 2021 13:59
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

good simplification 🤩

@ownclouders
Copy link
Contributor

💥 Acceptance tests Favorites failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15566/

webUIFavorites-favoritesFile-feature-58.png

webUIFavorites-favoritesFile-feature-58.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIBasic failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15566/

webUILogin-adminBlocksUser-feature-20.png

webUILogin-adminBlocksUser-feature-20.png

webUIPreview-imageMediaViewer-feature-140.png

webUIPreview-imageMediaViewer-feature-140.png

webUIPreview-imageMediaViewer-feature-158.png

webUIPreview-imageMediaViewer-feature-158.png

webUIPreview-imageMediaViewer-feature-83.png

webUIPreview-imageMediaViewer-feature-83.png

webUIPreview-imageMediaViewer-feature-90.png

webUIPreview-imageMediaViewer-feature-90.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIRename failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15566/

webUIRenameFiles-renameFiles-feature-186.png

webUIRenameFiles-renameFiles-feature-186.png

@pascalwengerter pascalwengerter force-pushed the 20052021_correctly-update-resource-in-filestable branch from 9c525bb to 6580cc8 Compare May 20, 2021 14:42
@ownclouders
Copy link
Contributor

💥 Acceptance tests Favorites failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15567/

webUIFavorites-favoritesFile-feature-58.png

webUIFavorites-favoritesFile-feature-58.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Favorites failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15572/

webUIFavorites-favoritesFile-feature-58.png

webUIFavorites-favoritesFile-feature-58.png

webUIFavorites-favoritesFile-feature-80.png

webUIFavorites-favoritesFile-feature-80.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIRename failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15572/

webUIRenameFiles-renameFiles-feature-186.png

webUIRenameFiles-renameFiles-feature-186.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Favorites failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15575/

webUIFavorites-favoritesFile-feature-58.png

webUIFavorites-favoritesFile-feature-58.png

webUIFavorites-favoritesFile-feature-80.png

webUIFavorites-favoritesFile-feature-80.png

@kulmann
Copy link
Member

kulmann commented May 21, 2021

Test scenario webUIFavorites/favoritesFile.feature:58 is an actual failure, not a random one. Needs to be fixed. It's succeeding on current master.

@pascalwengerter
Copy link
Contributor Author

Test scenario webUIFavorites/favoritesFile.feature:58 is an actual failure, not a random one. Needs to be fixed. It's succeeding on current master.

Thanks, will take care of this now!

@pascalwengerter
Copy link
Contributor Author

Test scenario webUIFavorites/favoritesFile.feature:58 is an actual failure, not a random one. Needs to be fixed. It's succeeding on current master.

Thanks, will take care of this now!

Can't really figure why this should be an actual failure 🤔 the CI is waiting (and then timing out) for .files-empty which I can see when navigation to the page myself - and the changes in this PR don't touch anything related to it AFAIK

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIRename failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15583/

webUIRenameFiles-renameFiles-feature-186.png

webUIRenameFiles-renameFiles-feature-186.png

@kulmann
Copy link
Member

kulmann commented May 21, 2021

Test scenario webUIFavorites/favoritesFile.feature:58 is an actual failure, not a random one. Needs to be fixed. It's succeeding on current master.

Thanks, will take care of this now!

Can't really figure why this should be an actual failure 🤔 the CI is waiting (and then timing out) for .files-empty which I can see when navigation to the page myself - and the changes in this PR don't touch anything related to it AFAIK

When I run the test locally I see a file in the favorites list. No idea why though. But it has to be related to the changes. UDATE_RESOURCE didn't do anything before if the id of the resource in the params was not found. Maybe that's the case for some favorites list update action and then it now inserts it while it shouldn't. Or a test (fixture?) bug related to this which didn't surface before.

@ownclouders
Copy link
Contributor

💥 Acceptance tests Favorites failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15586/

webUIFavorites-favoritesFile-feature-80.png

webUIFavorites-favoritesFile-feature-80.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIRename failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15586/

webUIRenameFiles-renameFiles-feature-186.png

webUIRenameFiles-renameFiles-feature-186.png

@kulmann kulmann force-pushed the 20052021_correctly-update-resource-in-filestable branch from 6580cc8 to d7c2371 Compare May 26, 2021 18:59
@kulmann
Copy link
Member

kulmann commented May 26, 2021

The issue why the test is failing is a sneaky bug, that was already pointed out by @labkode in #5085

You can reproduce it by clicking very fast on the All Files menu item, not waiting until it's fully rendered but directly clicking on Favorites again. What you can observe then, is that the files that have async preview requests will appear in the Favorites view. Because they get inserted now with this PR (as it switches from update to upsert for the preview responses).

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUIRename failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/web/15730/

webUIRenameFiles-renameFiles-feature-186.png

webUIRenameFiles-renameFiles-feature-186.png

@fschade fschade mentioned this pull request May 28, 2021
@fschade
Copy link
Contributor

fschade commented May 28, 2021

the request cancelation bug is tracked in #5163

@kulmann
Copy link
Member

kulmann commented Jun 11, 2021

Re-introduced the UPDATE_RESOURCE store mutation, because in most of the situations we don't want to allow an insert. But for the upload scenarios the UPSERT_RESOURCE makes sense and solves the bug with two versions of the same file being displayed as two files in the list.

@kulmann kulmann force-pushed the 20052021_correctly-update-resource-in-filestable branch from 82a442a to 7b0c3d8 Compare June 11, 2021 09:04
@sonarcloud
Copy link

sonarcloud bot commented Jun 11, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.2% 81.2% Coverage
5.3% 5.3% Duplication

@micbar
Copy link
Contributor

micbar commented Jun 12, 2021

❤️ Unit tests

Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

👍 LGTM

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.

Creating a new version of a file shows it duplicated in the file list (until page reload)
5 participants