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

Display current user in collaborators list #3060

Merged
merged 1 commit into from
Feb 20, 2020

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented Feb 18, 2020

How Has This Been Tested?

  • test environment: drone

Screenshots (if appropriate):

image

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:

Open tasks:

  • Add acceptance tests

@LukasHirt LukasHirt self-assigned this Feb 18, 2020
@LukasHirt LukasHirt force-pushed the feature/current-user-in-collaborators branch from dd3a780 to ca8523f Compare February 18, 2020 13:40
@LukasHirt LukasHirt added Status:Needs-Review Needs review from a maintainer and removed Status:In-Progress labels Feb 18, 2020
@LukasHirt LukasHirt marked this pull request as ready for review February 18, 2020 13:41
@LukasHirt LukasHirt requested a review from PVince81 February 18, 2020 13:42
@LukasHirt LukasHirt force-pushed the feature/current-user-in-collaborators branch from ca8523f to 2f9c152 Compare February 18, 2020 14:28
@LukasHirt
Copy link
Collaborator Author

Right, now need to explain to all the sharing scenarios that it's not wrong that there is one extra item in the list 🤦‍♂

@LukasHirt LukasHirt added Status:In-Progress and removed Status:Needs-Review Needs review from a maintainer labels Feb 18, 2020
@LukasHirt LukasHirt force-pushed the feature/current-user-in-collaborators branch 2 times, most recently from c78f71f to ded609c Compare February 19, 2020 10:23
@@ -47,8 +47,7 @@ Feature: Sharing files with internal groups with permissions
| file_target | /lorem (2).txt |
| item_type | file |
| permissions | <actual-permissions> |
But user "User One" should not be listed in the collaborators list on the webUI
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed because this step is no longer supposed to be true

@@ -48,8 +48,7 @@ Feature: Sharing folders with internal groups with role as advanced permissions
| file_target | /simple-folder (2) |
| item_type | folder |
| permissions | <actual-permissions> |
But user "User One" should not be listed in the collaborators list on the webUI
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed because this step is no longer supposed to be true

@@ -48,8 +48,7 @@ Feature: Sharing folders with internal groups with different roles and permissio
| file_target | /simple-folder (2) |
| item_type | folder |
| permissions | <actual-permissions> |
But user "User One" should not be listed in the collaborators list on the webUI
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed because this step is no longer supposed to be true

@LukasHirt LukasHirt added Status:Needs-Review Needs review from a maintainer and removed Status:In-Progress labels Feb 19, 2020
@LukasHirt LukasHirt force-pushed the feature/current-user-in-collaborators branch from ded609c to 711e1bc Compare February 19, 2020 14:11
@@ -563,3 +563,22 @@ Feature: Sharing files and folders with internal users
And the user opens the share dialog for folder "simple-folder" using the webUI
Then user "User Two" should be listed without additional info in the collaborators list on the webUI

Scenario: collaborators list contains the current user when he is an owner
Copy link
Contributor

@PVince81 PVince81 Feb 20, 2020

Choose a reason for hiding this comment

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

please always use the singular they: https://en.wikipedia.org/wiki/Singular_they

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adjusted all the scenarios titles. Pls re-check if I got it right 😉

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 see comments

@LukasHirt LukasHirt force-pushed the feature/current-user-in-collaborators branch from 711e1bc to 9556412 Compare February 20, 2020 10:22
@LukasHirt
Copy link
Collaborator Author

@PVince81 Addressed comments. Pls take a look one more time 🙂

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81 PVince81 merged commit 00768e4 into master Feb 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the feature/current-user-in-collaborators branch February 20, 2020 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants