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

[full-ci] StatusIndicators in ResourceTable Followup #6612

Merged
merged 9 commits into from
Mar 28, 2022

Conversation

lookacat
Copy link
Contributor

@lookacat lookacat commented Mar 17, 2022

Description

See #5976

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Mar 17, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@lookacat lookacat marked this pull request as ready for review March 17, 2022 12:05
@lookacat lookacat changed the title StatusIndicators in ResourceTable Followup [full-ci] StatusIndicators in ResourceTable Followup Mar 17, 2022
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.

Please rebase to make e2e tests pass again. Also see comment for regarding changelog, I think the removal of links from shared with others deserves its own changelog item because it has nothing to do with the separate column for share indicators.

changelog/unreleased/enhancement-move-share-indicators Outdated Show resolved Hide resolved
@ownclouders
Copy link
Contributor

ownclouders commented Mar 21, 2022

Results for oC10iPhone1 https://drone.owncloud.com/owncloud/web/24106/48/1
The following scenarios passed on retry:

  • webUIPrivateLinks/accessingPrivateLinks.feature:9

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.

Needs a rebase, but looks good otherwise 👍

@pascalwengerter
Copy link
Contributor

pascalwengerter commented Mar 22, 2022

Running the latest CI failure (https://drone.owncloud.com/owncloud/web/24011/44/20) locally as SCREEN_RESOLUTION=768x1024 yarn test:acceptance:oc10 features/webUIRestrictSharing/restrictReSharing.feature:23 (pasting it here in case I miss something obvious) fails with "resource can't be favorited in the server" - which is not what the CI fails with. @kulmann @fschade any ideas?

@pascalwengerter
Copy link
Contributor

Running the latest CI failure (https://drone.owncloud.com/owncloud/web/24011/44/20) locally as SCREEN_RESOLUTION=768x1024 yarn test:acceptance:oc10 features/webUIRestrictSharing/restrictReSharing.feature:23 (pasting it here in case I miss something obvious) fails with "resource can't be favorited in the server" - which is not what the CI fails with. @kulmann @fschade any ideas?

Tried dropping volumes & pulling newer images, problem stays

@kulmann
Copy link
Member

kulmann commented Mar 23, 2022

Running the latest CI failure (https://drone.owncloud.com/owncloud/web/24011/44/20) locally as SCREEN_RESOLUTION=768x1024 yarn test:acceptance:oc10 features/webUIRestrictSharing/restrictReSharing.feature:23 (pasting it here in case I miss something obvious) fails with "resource can't be favorited in the server" - which is not what the CI fails with. @kulmann @fschade any ideas?

Tried dropping volumes & pulling newer images, problem stays

might be a timing issue, that the share is not yet accepted. try moving the line that sets the favorite some lines down (below the other middleware step).

@kulmann kulmann force-pushed the status-indicators-followup branch 2 times, most recently from a4ab142 to a64eb8b Compare March 24, 2022 23:22
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Rebased, and highly in favor of merging this one if CI gets green finally. There's some usability things to think of (e.g. clicking on share giver in the shared-with-me view leads to the sharing tab on the sidebar that simply reads "No permissions to share file/folder", but we can fix that at a later stage)

@kulmann kulmann force-pushed the status-indicators-followup branch 2 times, most recently from 5f635b6 to a2e69ac Compare March 28, 2022 13:31
lookacat and others added 9 commits March 28, 2022 15:42
Update changelog, Update tests and snapshots, Linting

Fix share avatar colors
Check if any resource in table has indicators, update snapshot
…Update snapshots

Update snapshots

update snapshots

Fix acceptance tests

make linter happy

Fix public link share acceptance tests

Rebase & make acceptance test rely on web ui actions
Idea was to use the web ui for accepting the share, but this fails on
mobile resolutions where the status column doesn't fit on the screen.
@sonarcloud
Copy link

sonarcloud bot commented Mar 28, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

84.6% 84.6% Coverage
2.4% 2.4% Duplication

@kulmann kulmann merged commit ef7a66d into master Mar 28, 2022
@delete-merged-branch delete-merged-branch bot deleted the status-indicators-followup branch March 28, 2022 14:55
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.

Move share indicators into their own column
4 participants