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

[tests-only] tests: refactor test script for removing permission from share #8134

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

SwikritiT
Copy link
Contributor

Description

Previously I fetched the permissionId with the extra request but the permissionId is what we knew to be shareId so we can store the shareId directly from the share invite response and use it from the stored array. This removes the necessity for extra request.

Related Issue

#8008

Copy link

update-docs bot commented Jan 5, 2024

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.

@SwikritiT SwikritiT changed the title tests: refactor test script for removing permission from share [tests-only] tests: refactor test script for removing permission from share Jan 5, 2024
@SwikritiT SwikritiT self-assigned this Jan 5, 2024
@SwikritiT SwikritiT marked this pull request as ready for review January 5, 2024 07:02
@@ -179,11 +200,22 @@ public function getSharesEndpointPath(?string $postfix = ''):string {
public function shareNgGetLastCreatedLinkShareID(): string {
$lastResponse = $this->shareNgGetLastCreatedLinkShare();
if (!isset($this->getJsonDecodedResponse($lastResponse)['id'])) {
throw new Error('Response did not contain share id ' . $this->getJsonDecodedResponse($lastResponse)['id'] . ' for the created public link');
Copy link
Contributor Author

@SwikritiT SwikritiT Jan 5, 2024

Choose a reason for hiding this comment

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

removed this because if $this->getJsonDecodedResponse($lastResponse)['id'] is not set it'll probably come undefined. so the error message might not make use of that

Copy link
Contributor

@grgprarup grgprarup left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

others are looking good.

@saw-jan saw-jan force-pushed the tests/refactor-remove-share-permission branch 2 times, most recently from 1cb694a to 0358930 Compare January 8, 2024 08:10
@SwikritiT SwikritiT force-pushed the tests/refactor-remove-share-permission branch from 0358930 to 3c383a3 Compare January 9, 2024 04:00
@SwikritiT SwikritiT force-pushed the tests/refactor-remove-share-permission branch from 3c383a3 to 87e5116 Compare January 9, 2024 05:33
Copy link

sonarqubecloud bot commented Jan 9, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@SwikritiT SwikritiT merged commit 954a1c0 into stable-5.0 Jan 9, 2024
4 checks passed
@delete-merged-branch delete-merged-branch bot deleted the tests/refactor-remove-share-permission branch January 9, 2024 08:11
ownclouders pushed a commit that referenced this pull request Jan 9, 2024
… share (#8134)

* tests: refactor test script for removing permission from share

* address reviews

---------

Co-authored-by: Saw-jan <[email protected]>
saw-jan added a commit that referenced this pull request Jan 16, 2024
… share (#8134)

* tests: refactor test script for removing permission from share

* address reviews

---------

Co-authored-by: Saw-jan <[email protected]>
nirajacharya2 pushed a commit that referenced this pull request Jan 22, 2024
… share (#8134)

* tests: refactor test script for removing permission from share

* address reviews

---------

Co-authored-by: Saw-jan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants