User can hide liked bookmarks from other users using server-side privacy#45
User can hide liked bookmarks from other users using server-side privacy#45lymichelle21 wants to merge 9 commits into
Conversation
| import android.net.Uri; | ||
| import android.os.Bundle; | ||
| import android.provider.MediaStore; | ||
| import android.util.Log; |
There was a problem hiding this comment.
Seems we are not logging anything so could you please remove this import.
| @@ -1,5 +1,7 @@ | |||
| package com.capstone.event_finder.activities; | |||
|
|
|||
| import static androidx.constraintlayout.helper.widget.MotionEffect.TAG; | |||
There was a problem hiding this comment.
Also check whether we need this or not
bsmondal
left a comment
bsmondal
left a comment
There was a problem hiding this comment.
Looks good to me. Please have a quick look into my comment
| } else { | ||
| bookmarkAcl.setPublicReadAccess(true); | ||
| } | ||
| saveBookmark(eventId, eventCategory, currentUser, bookmarkAcl); |
There was a problem hiding this comment.
✅ Thanks for giving ACL a try to ensure sever-side security of a user's private bookmarks.
| user.put("event_categories_string", allInterestCategories); | ||
| user.put("profile_image", file); | ||
|
|
||
| user.put("private", isPrivate); |
There was a problem hiding this comment.
❓ What happens to existing bookmarks when the user changes their privacy? Should we be looping through all the user's bookmarks and updating their ACL? (Remember we can't completely rely on the client-side check you're making in PosterProfileActivity)
| private final EventApi eventApi; | ||
| private final List<Bookmark> userBookmarks = new ArrayList<>(); | ||
| private final MutableLiveData<List<Event>> bookmarkList; | ||
| private final ArrayList<String> bookmarkIds = new ArrayList<>(); |
There was a problem hiding this comment.
✅ Thanks for making this change, it might seem trivial but will help avoid troubles in the future.
The "Clean Code" book I recommended has more information on this under "Chapter 3: Functions" under "Have No Side Effects".
| } | ||
|
|
||
| private Collection<? extends Event> convertBookmarksToList(JsonArray result) { | ||
| private Collection<? extends Event> convertToList(JsonArray result) { |
There was a problem hiding this comment.
✅ Thanks for making this change, in this case, less code makes this cleaner!
3 participants
New stuff:
Screenshots: