Consolidate user/group share actions into single dropdown #36587

felixheidecke · 2019-12-13T14:10:57Z

Rework user/group share actions. Concerning https://github.com/owncloud/enterprise/issues/2001

felixheidecke · 2019-12-13T14:13:00Z

@PVince81 I added the "silent model update" in order to prevent an immediate redraw wich closed the dropdown while changing stuff:
https://github.com/owncloud/core/pull/36587/files#diff-2e1f33af47835fc7dab530c048c95f9aR702

I hope that doesn't collide somewhere else. Tests are fine.

felixheidecke · 2019-12-13T14:20:11Z

I consider allowing the redraw on removing or adding the expiration date, so the little ⏱ immediately appears/disappears.

phil-davis · 2019-12-13T14:40:10Z

Seems to work nicely with my 15 minutes manual testing.

An unusual sequence that is odd is:

enable, enforce and set the default/enforced days to 30 for user shares
create a user share - it gets expiry date 30 days from now by default, good
change the default/enforced days to 15 for user shares
refresh the files page, redisplay the share you created. The expiry date shows as only 15 days in the future. (that might be "a good thing" if it really got reset to that)
change the default/enforced days to 30 or more for user shares
refresh the files page, redisplay the share you created. The expiry date shows as 30 days in the future. (which is the value that it really has in the server)

So when displaying an existing share, if the share has an expiry date past the current enforced limit, then the displayed expiry date is just the current enforced limit. (which is not a true indication of what is in the server database)

phil-davis · 2019-12-13T14:42:05Z

The UI tests will need refactoring for the new/changed behaviour here. Without having looked, I guess that it will not just be the Gherkin scenarios, but the placement of UI date elements has changed so xpath... in page objects probably needs sorting out.

phil-davis · 2019-12-13T14:43:37Z

I added some labels, because I guess QA-team will be helping with adjusting the UI acceptance tests.

felixheidecke · 2019-12-13T15:24:20Z

refresh the files page, redisplay the share you created. The expiry date shows as only 15 days in the future. (that might be "a good thing" if it really got reset to that)

hmmm, I guess that's because the jQuery Datepicker sets a maxDate based on the enforced default expiration. Hmmm...

phil-davis · 2019-12-16T03:38:25Z

hmmm, I guess that's because the jQuery Datepicker sets a maxDate based on the enforced default expiration. Hmmm...

I don't think it's a big thing - it only happens if the admin has set the default days to be long, then later reduces it.

codecov · 2019-12-16T04:55:38Z

Codecov Report

Merging #36587 into feature/user-group-share-expiration will increase coverage by <.01%.
The diff coverage is 72.72%.

@@                            Coverage Diff                            @@
##             feature/user-group-share-expiration   #36587      +/-   ##
=========================================================================
+ Coverage                                  64.64%   64.65%   +<.01%     
  Complexity                                 19078    19078              
=========================================================================
  Files                                       1269     1269              
  Lines                                      74668    74666       -2     
  Branches                                    1322     1319       -3     
=========================================================================
+ Hits                                       48269    48272       +3     
+ Misses                                     26007    26005       -2     
+ Partials                                     392      389       -3

Flag	Coverage Δ	Complexity Δ
#javascript	`54.08% <72.72%> (+0.05%)`	`0 <0> (ø)`	⬇️
#phpunit	`65.82% <ø> (ø)`	`19078 <ø> (ø)`	⬇️

Impacted Files	Coverage Δ	Complexity Δ
core/js/shareitemmodel.js	`80.31% <100%> (+0.05%)`	`0 <0> (ø)`	⬇️
core/js/sharedialogshareelistview.js	`73.23% <62.5%> (-0.56%)`	`0 <0> (ø)`
core/js/shareconfigmodel.js	`83.67% <0%> (+10.2%)`	`0% <0%> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cac2f2...bd25355. Read the comment docs.

dpakach · 2019-12-16T04:55:52Z

@felixheidecke, I just pushed a fix for the CI, Haven't run all the tests but they should be good now.

…is off

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php

micbar

Code Style

tests/acceptance/features/webUISharingInternalGroups/shareWithGroupUsingExpirationDate.feature

tests/acceptance/features/webUISharingInternalUsers/shareWithUserUsingExpirationDate.feature

skshetry

Just one thing, rest looked good to me. :)

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php

phil-davis · 2019-12-16T10:29:35Z

https://drone.owncloud.com/owncloud/core/22008/81/14

    Then the expiration date input field should be "+30 days" for the group "grp1" in the share dialog                                                                                  # WebUISharingContext::expirationDateShouldBe()
      Failed asserting that '15-01-2020' matches expected 1579083368.

in a few places.
Some problem with the expirationDateShouldBe() comparison code
@dpakach

tests/acceptance/features/bootstrap/WebUISharingContext.php

phil-davis · 2019-12-17T05:56:38Z

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php

 			//the additional permission disappear again after they are changed
 			//so we need to open them again and again


I guess this comment is no longer true?

phil-davis · 2019-12-17T06:00:42Z

This is failing: https://drone.owncloud.com/owncloud/core/22028/93/14

phil-davis · 2019-12-17T07:49:26Z

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php

+	}
+
+	/**
+	 * open the dropdown for share actions in the sidebar


Suggested change

* open the dropdown for share actions in the sidebar

* close the dropdown for share actions in the sidebar

I will fix this after merging, when I am rebasing the PR to which this gets merged.

phil-davis · 2019-12-17T07:54:13Z

I will merge this to feature/user-group-share-expiration when CI goes green. Then I will rebase/squash/sort-out the commits in feature/user-group-share-expiration. Then we can do a "final" review there.

dpakach · 2019-12-17T08:03:26Z

@phil-davis one test is failing in drone but passing locally, I will try and find what's causing that.

phil-davis · 2019-12-17T08:06:58Z

@phil-davis one test is failing in drone but passing locally, I will try and find what's causing that.

OK - if you have to push another commit, then you may as well also fix the test at SharingDialog.php line 133

dpakach · 2019-12-17T09:08:04Z

@felixheidecke tests/acceptance/features/webUIRestrictSharing/restrictReSharing.feature:24 was failing in firefox. Currently we use older version of firefox (58.0). The dropdown doesn't works on that version but works on the recent one ( I tested firefox 71 and it worked). Does the older version needs to be supported?

felixheidecke · 2019-12-18T14:44:31Z

@felixheidecke tests/acceptance/features/webUIRestrictSharing/restrictReSharing.feature:24 was failing in firefox. Currently, we use older version of firefox (58.0). The dropdown doesn't works on that version but works on the recent one ( I tested firefox 71 and it worked). Does the older version needs to be supported?

@dpakach Looks like @phil-davis decided that it doesn't need to be supported ;-)

phil-davis · 2019-12-18T15:24:19Z

Yes, we talked about it. We manually tested in the latest Firefox and it works. We do not support every version of Firefox - so it would be "a bit random" to insist on fixing something on Firefox58 and other things might or might not work on Firefox42,57,59,60,61,...

WIP

aa62cca

felixheidecke added the 3 - To Review label Dec 13, 2019

felixheidecke added this to the development milestone Dec 13, 2019

felixheidecke requested review from phil-davis and micbar December 13, 2019 14:10

felixheidecke self-assigned this Dec 13, 2019

phil-davis added dev:acceptance-tests QA:team labels Dec 13, 2019

dpakach self-assigned this Dec 16, 2019

Fix The sharingDialog PageObject for changed share actions dropdown

95f1afd

Fix tests and add scenarios for setting expiry date when enforcement …

69ecc01

…is off

micbar reviewed Dec 16, 2019

View reviewed changes

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php Show resolved Hide resolved

micbar reviewed Dec 16, 2019

View reviewed changes

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php Show resolved Hide resolved

micbar reviewed Dec 16, 2019

View reviewed changes

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php Show resolved Hide resolved

micbar reviewed Dec 16, 2019

View reviewed changes

phil-davis reviewed Dec 16, 2019

View reviewed changes

skshetry reviewed Dec 16, 2019

View reviewed changes

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php Outdated Show resolved Hide resolved

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php Outdated Show resolved Hide resolved

Fix code style and extend expiration date scenarios

1a1b41f

phil-davis reviewed Dec 16, 2019

View reviewed changes

tests/acceptance/features/lib/FilesPageElement/SharingDialog.php Show resolved Hide resolved

skshetry reviewed Dec 16, 2019

View reviewed changes

tests/acceptance/features/bootstrap/WebUISharingContext.php Outdated Show resolved Hide resolved

dpakach and others added 3 commits December 16, 2019 17:07

Fix checking expiration date value in UI tests

9448730

openShareActionsDropDown before clearExpirationDateFor

58a2fe1

Adjust federation sharing test for change in setting permissions

f52ed88

phil-davis reviewed Dec 17, 2019

View reviewed changes

fix remaining failing tests

3a090a1

phil-davis self-requested a review December 17, 2019 07:47

phil-davis approved these changes Dec 17, 2019

View reviewed changes

phil-davis added 4 - To release and removed 3 - To Review labels Dec 17, 2019

Skip test failing on older version of firefox

bd25355

phil-davis approved these changes Dec 17, 2019

View reviewed changes

phil-davis merged commit 47c4ef7 into feature/user-group-share-expiration Dec 17, 2019

delete-merged-branch bot deleted the feature/user-group-share-expiration-consolidation branch December 17, 2019 10:33

phil-davis mentioned this pull request Dec 17, 2019

[For 10.4] Include expiration option for user and group shares #36573

Merged

15 tasks

felixheidecke mentioned this pull request Dec 19, 2019

Add Changelog 36587 #36617

Merged

davitol mentioned this pull request Jan 9, 2020

Tracking test results for ownCloud 10.4.0 #36732

Closed

39 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate user/group share actions into single dropdown #36587

Consolidate user/group share actions into single dropdown #36587

felixheidecke commented Dec 13, 2019

felixheidecke commented Dec 13, 2019 •

edited

Loading

felixheidecke commented Dec 13, 2019

phil-davis commented Dec 13, 2019

phil-davis commented Dec 13, 2019

phil-davis commented Dec 13, 2019

felixheidecke commented Dec 13, 2019

phil-davis commented Dec 16, 2019

codecov bot commented Dec 16, 2019 •

edited

Loading

dpakach commented Dec 16, 2019

micbar left a comment

skshetry left a comment

phil-davis commented Dec 16, 2019

phil-davis Dec 17, 2019

phil-davis commented Dec 17, 2019

phil-davis Dec 17, 2019

phil-davis commented Dec 17, 2019

dpakach commented Dec 17, 2019

phil-davis commented Dec 17, 2019

dpakach commented Dec 17, 2019

felixheidecke commented Dec 18, 2019

phil-davis commented Dec 18, 2019

		//the additional permission disappear again after they are changed
		//so we need to open them again and again

	* open the dropdown for share actions in the sidebar
	* close the dropdown for share actions in the sidebar

Consolidate user/group share actions into single dropdown #36587

Consolidate user/group share actions into single dropdown #36587

Conversation

felixheidecke commented Dec 13, 2019

felixheidecke commented Dec 13, 2019 • edited Loading

felixheidecke commented Dec 13, 2019

phil-davis commented Dec 13, 2019

phil-davis commented Dec 13, 2019

phil-davis commented Dec 13, 2019

felixheidecke commented Dec 13, 2019

phil-davis commented Dec 16, 2019

codecov bot commented Dec 16, 2019 • edited Loading

Codecov Report

dpakach commented Dec 16, 2019

micbar left a comment

Choose a reason for hiding this comment

skshetry left a comment

Choose a reason for hiding this comment

phil-davis commented Dec 16, 2019

phil-davis Dec 17, 2019

Choose a reason for hiding this comment

phil-davis commented Dec 17, 2019

phil-davis Dec 17, 2019

Choose a reason for hiding this comment

phil-davis commented Dec 17, 2019

dpakach commented Dec 17, 2019

phil-davis commented Dec 17, 2019

dpakach commented Dec 17, 2019

felixheidecke commented Dec 18, 2019

phil-davis commented Dec 18, 2019

felixheidecke commented Dec 13, 2019 •

edited

Loading

codecov bot commented Dec 16, 2019 •

edited

Loading