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.

Sign up for GitHub

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

Jump to bottom

Fix Restore file to read-only node (DecomposedFS) #1913

Merged
merged 11 commits into from
Aug 20, 2021
Merged

Fix Restore file to read-only node (DecomposedFS) #1913

merged 11 commits into from
Aug 20, 2021

Conversation

refs
Copy link
Member

@refs refs commented Jul 22, 2021 •
edited
Loading

test fixed?
apiTrashbin/trashbinSharingToShares.feature:82 ✅
apiTrashbin/trashbinSharingToShares.feature:83 ✅
apiTrashbin/trashbinSharingToShares.feature:102 ✅
apiTrashbin/trashbinSharingToShares.feature:103 ✅

@refs refs requested a review from labkode as a code owner July 22, 2021 19:33
@update-docs
Copy link

update-docs bot commented Jul 22, 2021

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.

@refs refs requested review from C0rby, butonic and ishank011 July 22, 2021 19:35
@lgtm-com
Copy link

lgtm-com bot commented Jul 22, 2021

This pull request fixes 13 alerts when merging f151d38 into 4475f43 - view on LGTM.com

fixed alerts:

  • 13 for Uncontrolled data used in path expression

@lgtm-com
Copy link

lgtm-com bot commented Jul 23, 2021

This pull request fixes 13 alerts when merging 6bd2f12 into 4475f43 - view on LGTM.com

fixed alerts:

  • 13 for Uncontrolled data used in path expression

@refs refs force-pushed the fix-readonly-restores branch from 6bd2f12 to b9364ec Compare July 23, 2021 10:40
@lgtm-com
Copy link

lgtm-com bot commented Jul 23, 2021

This pull request fixes 13 alerts when merging b9364ec into 3136771 - view on LGTM.com

fixed alerts:

  • 13 for Uncontrolled data used in path expression

@lgtm-com
Copy link

lgtm-com bot commented Jul 23, 2021

This pull request fixes 13 alerts when merging b69d19a into 3136771 - view on LGTM.com

fixed alerts:

  • 13 for Uncontrolled data used in path expression

C0rby
C0rby previously approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@C0rby C0rby left a comment

Choose a reason for hiding this comment

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

I think this looks good. But what is with @butonic's remark: #1913 (comment)?

@refs
Copy link
Member Author

refs commented Jul 23, 2021 •
edited
Loading

I think this looks good. But what is with @butonic's remark: #1913 (comment)?

It's a bigger cookie to eat 🍪 I made that logic strict by design and enforce a regexp so the format of the resourceid is clear. But @butonic comments are very relevant, but have to be addressed at a higher level since resourceids are pretty much the wild wild west :D

@refs refs changed the title [WIP] Fix Restore file to read-only node (DecomposedFS) Fix Restore file to read-only node (DecomposedFS) Jul 27, 2021
@butonic
Copy link
Contributor

butonic commented Jul 29, 2021 •
edited
Loading

@refs please rebase and drop the regex based UUID format check.

@refs
Copy link
Member Author

refs commented Aug 18, 2021

@labkode 🍰 ready for review and mergie mergie 👯

@labkode labkode merged commit a965121 into cs3org:master Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@butonic butonic butonic left review comments

@C0rby C0rby C0rby approved these changes

@labkode labkode labkode approved these changes

@ishank011 ishank011 Awaiting requested review from ishank011

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@refs @butonic @labkode @C0rby