Skip to content

Conversation

@AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented Jun 1, 2018

At the moment the server supports setting a password for a share link and for an email share. The fact that a share is actually password protected isn't reflected in the OCShare object.
For links that has been fine since if shared-with is set is is password protected (even though this sounds strange...). This mechanism however won't work for email shares thus the client using the library should be able to parse/get/set this info and later store this fact to the database.

mUserId = source.readLong();
mRemoteId = source.readLong();
mShareLink = source.readString();
mIsPasswordProtected = source.readInt() == 0;
Copy link
Member

Choose a reason for hiding this comment

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

We are writing a 1 if mIsPasswortProtected is true in line 346, but compare with 0?
So if mIsPasswortProteced = true, then
on write: dest.writeInt(1).
on read: source.readInt() == 0 --> 1 == 0 --> false --> mIsPasswordProtected = false

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Jun 4, 2018

👎 (this feels so harsh)

Rejected with PullApprove

@AndyScherzinger
Copy link
Member Author

👎 (this feels so harsh)

All good :) Fixed your comment :) -> 431cf90

@nextcloud nextcloud deleted a comment Jun 4, 2018
@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Jun 5, 2018

@mario @tobiasKaminsky please review, this PR is needed by nextcloud/android#2671

@tobiasKaminsky well, please re-review since I fixed your comment :)

@nextcloud nextcloud deleted a comment Jun 5, 2018
@AndyScherzinger
Copy link
Member Author

AWAITING 2ND PEER REVIEW SINCE 8 DAYS // AWAITING RE-REVIEW SINCE 5 DAYS

@mario @tobiasKaminsky please review, this PR is needed by nextcloud/android#2671

@tobiasKaminsky well, please re-review since I fixed your comment :)

@mario
Copy link
Contributor

mario commented Jun 9, 2018

👍

Approved with PullApprove

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Jun 12, 2018

👍

Approved with PullApprove

@AndyScherzinger AndyScherzinger merged commit 5999ed9 into master Jun 12, 2018
@AndyScherzinger AndyScherzinger deleted the shareParameters branch June 12, 2018 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants