Test regression with public link not created when default expiration date set and enforced by skshetry · Pull Request #35594 · owncloud/core

skshetry · 2019-06-19T09:42:49Z

Description

This adds a regression test that public link was not getting created when default expiration date was set and enforced. This also tests the functionality of #34960.

Related Issue

Fixes Public link share creation no longer automatically sets expiration date #35550

Motivation and Context

How Has This Been Tested?

locally

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

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

Open tasks:

Backport (if applicable set "backport-request" label and remove when the backport was done)

skshetry · 2019-06-19T09:43:29Z

I am currently testing this without the fix in #35564. I'll just remove comments when the tests fail.

TODO:

Check that current tests pass.
Remove comments, and check that it fails.
Rebase, and check that it passes.
Backport to stable10
Backport to release-10.2.1

micbar · 2019-06-19T11:33:41Z

@skshetry Fix got merged in

master
stable10
release-10.2.1

codecov · 2019-06-20T08:39:43Z

Codecov Report

Merging #35594 into master will increase coverage by 0.29%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #35594      +/-   ##
============================================
+ Coverage     65.37%   65.67%   +0.29%     
+ Complexity    18775    18763      -12     
============================================
  Files          1222     1222              
  Lines         70898    70887      -11     
  Branches       1289     1289              
============================================
+ Hits          46353    46555     +202     
+ Misses        24167    23954     -213     
  Partials        378      378

Flag	Coverage Δ	Complexity Δ
#javascript	`53.7% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`67.04% <ø> (+0.32%)`	`18763 <ø> (-12)`	⬇️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Files/Cache/HomePropagator.php	`77.77% <0%> (-9.73%)`	`3% <0%> (ø)`
apps/files/lib/Command/Scan.php	`71.91% <0%> (-9.17%)`	`74% <0%> (ø)`
core/Command/Encryption/EncryptAll.php	`90.9% <0%> (-1.1%)`	`11% <0%> (-1%)`
core/Controller/LostController.php	`88.48% <0%> (-0.7%)`	`33% <0%> (-4%)`
...es_sharing/lib/Controller/Share20OcsController.php	`87.39% <0%> (-0.11%)`	`209% <0%> (-2%)`
apps/dav/lib/Connector/Sabre/File.php	`84.19% <0%> (+0.64%)`	`115% <0%> (ø)`	⬇️
lib/private/Files/Mount/MountPoint.php	`93.5% <0%> (+1.29%)`	`34% <0%> (ø)`	⬇️
lib/private/legacy/util.php	`68.19% <0%> (+2.55%)`	`240% <0%> (ø)`	⬇️
apps/files_versions/lib/Storage.php	`72.67% <0%> (+2.81%)`	`95% <0%> (ø)`	⬇️
lib/private/Files/Storage/Common.php	`85.15% <0%> (+3.33%)`	`141% <0%> (ø)`	⬇️
... and 13 more

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 192501e...f2fd57d. Read the comment docs.

codecov · 2019-06-20T08:40:22Z

Codecov Report

Merging #35594 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #35594   +/-   ##
=========================================
  Coverage     65.77%   65.77%           
  Complexity    18781    18781           
=========================================
  Files          1222     1222           
  Lines         70870    70870           
  Branches       1289     1289           
=========================================
  Hits          46618    46618           
  Misses        23874    23874           
  Partials        378      378

Flag	Coverage Δ	Complexity Δ
#javascript	`53.7% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`67.16% <ø> (ø)`	`18781 <ø> (ø)`	⬇️

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 29bbe99...3845c54. Read the comment docs.

skshetry · 2019-06-20T09:06:41Z

 	 */
 	public function theUserShouldSeeAnErrorMessageOnThPublicLinkPopupSaying($message) {
+		// update public-sharing popup, as it might not have been set previously.
+		$this->publicSharingPopup = $this->publicShareTab->updateSharingPopup($this->getSession());


There's no way to get the popup. It's not the best way, but I went with the easiest way.

paurakhsharma · 2019-06-21T07:04:35Z

 	}

+	/**
+	 * @When /^user "([^"]*)" gets the info of the last public link using the sharing API$/


I think this should go inside Sharing.php

I feel this is good-enough, because if I put this in Sharing.php:

The similar step will exist.

Other developers might use this step even though, they should be using other step (due to suggestion from autocompletion).

Might need to use xpath on most element (due to responseXml differences).

This is only needed for webUI stuffs. Keeping on webUI prevents step2 from happening and provide isolation.

Your argument does hold. api related calls for Sharing should be in Sharing.php, I agree.

paurakhsharma

Just a small comment, other looks good for me.

haribhandari07 · 2019-06-21T08:37:09Z

looks good

individual-it · 2019-06-21T11:02:35Z

 	}

+	/**
+	 * @When /^user "([^"]*)" gets the info of the last public link using the sharing API$/


why not using userGetsInfoOfLastShareUsingTheSharingApi()?

Creating public links does not set lastShareData which userGetsInfoOfLastShareUsingTheSharingApi uses. So, I have to manually set.

OK, you don't have the id of the share here, get it and then use userGetsInfoOfLastShareUsingTheSharingApi

individual-it

also fix aded639#diff-7a6686160e3f99113ae6c2c807e63a58R801

I think that is what you are stumbling about.
in isFieldInResponse we mostly use data from the last response $this->getResponseXml() but for the expiration date we get the data when the share was created $this->getServerLastShareTime() this is not correct if the share was created but an other response like
And the user gets the info of the last share using the sharing API was done afterwards

individual-it · 2019-06-21T11:40:31Z

 	}

+	/**
+	 * @When /^user "([^"]*)" gets the info of the last public link using the sharing API$/


OK, you don't have the id of the share here, get it and then use userGetsInfoOfLastShareUsingTheSharingApi

…et and enforced

phil-davis · 2019-07-01T06:36:46Z

@skshetry please make the stable10 backport.
Then I will look at what acceptance test changes are pending release-10.2.1 backport and make a PR for those. That will run all these scenarios against release-10.2.1

skshetry · 2019-07-01T06:54:03Z

Backport for stable10: #35703

skshetry added 2 - Developing QA:team Acceptance tests labels Jun 19, 2019

skshetry requested review from dpakach, haribhandari07 and paurakhsharma June 19, 2019 09:42

skshetry self-assigned this Jun 19, 2019

skshetry added the backport-request label Jun 19, 2019

dpakach reviewed Jun 20, 2019

View reviewed changes

Comment thread tests/acceptance/features/apiShareManagementBasic/createShare.feature Outdated

skshetry force-pushed the regression-35550 branch from d6b3876 to f2fd57d Compare June 20, 2019 08:39

skshetry added 3 - To Review and removed 2 - Developing labels Jun 20, 2019

skshetry force-pushed the regression-35550 branch 2 times, most recently from 4e315c5 to f8822d1 Compare June 20, 2019 09:03

skshetry commented Jun 20, 2019

View reviewed changes

skshetry requested review from dpakach, individual-it, karakayasemi and phil-davis June 20, 2019 09:29

karakayasemi approved these changes Jun 20, 2019

View reviewed changes

paurakhsharma reviewed Jun 21, 2019

View reviewed changes

individual-it requested changes Jun 21, 2019

View reviewed changes

skshetry force-pushed the regression-35550 branch from f8822d1 to 7a46531 Compare June 21, 2019 11:16

individual-it requested changes Jun 21, 2019

View reviewed changes

skshetry added 2 - Developing and removed 3 - To Review labels Jun 24, 2019

skshetry force-pushed the regression-35550 branch 3 times, most recently from d6cc58d to affec79 Compare June 24, 2019 16:54

individual-it requested changes Jun 25, 2019

View reviewed changes

Comment thread tests/acceptance/features/apiShareManagementBasic/createShare.feature

skshetry force-pushed the regression-35550 branch 4 times, most recently from fc07092 to d5083fb Compare June 26, 2019 11:07

skshetry requested a review from individual-it June 26, 2019 11:15

skshetry added 3 - To Review and removed 2 - Developing labels Jun 28, 2019

skshetry force-pushed the regression-35550 branch from d5083fb to c19f092 Compare July 1, 2019 05:52

skshetry added 2 commits July 1, 2019 11:56

Test regression public link not creating on default expiration date s…

56b3066

…et and enforced

Test for happy path as it's fixed

3845c54

skshetry force-pushed the regression-35550 branch from c19f092 to 3845c54 Compare July 1, 2019 06:12

individual-it approved these changes Jul 1, 2019

View reviewed changes

phil-davis approved these changes Jul 1, 2019

View reviewed changes

phil-davis added 4 - To release and removed 3 - To Review labels Jul 1, 2019

skshetry mentioned this pull request Jul 1, 2019

[stable10] Test regression with public link not created when default expiration date set and enforced #35703

Merged

11 tasks

skshetry merged commit f4079cf into master Jul 1, 2019

delete-merged-branch Bot deleted the regression-35550 branch July 1, 2019 07:20

phil-davis removed the backport-request label Jul 1, 2019

Uh oh!

Conversation

skshetry commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Open tasks:

Uh oh!

skshetry commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

micbar commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 20, 2019

Codecov Report

Uh oh!

codecov Bot commented Jun 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skshetry Jun 20, 2019

Choose a reason for hiding this comment

Uh oh!

paurakhsharma Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

skshetry Jun 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paurakhsharma left a comment

Choose a reason for hiding this comment

Uh oh!

haribhandari07 commented Jun 21, 2019

Uh oh!

Uh oh!

individual-it Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

skshetry Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

individual-it Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

individual-it left a comment

Choose a reason for hiding this comment

Uh oh!

individual-it Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phil-davis commented Jul 1, 2019

Uh oh!

skshetry commented Jul 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

skshetry commented Jun 19, 2019 •

edited

Loading

skshetry commented Jun 19, 2019 •

edited

Loading

micbar commented Jun 19, 2019 •

edited

Loading

codecov Bot commented Jun 20, 2019 •

edited

Loading

skshetry Jun 21, 2019 •

edited

Loading