-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[MBL-1026] Filter comments from blocked users #1883
Changes from all commits
ec8ec5d
17be120
0f99421
5a1d4bb
8d87688
ba910f8
e5e7e75
1d6ca14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ public protocol CommentCellViewModelOutputs { | |
var authorBadge: Signal<Comment.AuthorBadge, Never> { get } | ||
|
||
/// Emits a url to the comment author's image. | ||
var authorImageURL: Signal<URL, Never> { get } | ||
var authorImageURL: Signal<URL?, Never> { get } | ||
|
||
/// Emits text containing author's fullname or username. | ||
var authorName: Signal<String, Never> { get } | ||
|
@@ -85,13 +85,32 @@ public final class CommentCellViewModel: | |
.map { comment, _ in comment } | ||
|
||
self.authorImageURL = comment | ||
.map(\.author.imageUrl) | ||
.map { | ||
if $0.author.isBlocked { | ||
return "" // Use placeholder avatar instead of URL. | ||
} else { | ||
return $0.author.imageUrl | ||
} | ||
} | ||
.map(URL.init) | ||
.skipNil() | ||
|
||
self.body = comment.map(\.body) | ||
self.body = comment.map { | ||
if $0.author.isBlocked { | ||
// TODO(MBL-1026): Use translated string once available. | ||
return "This user has been blocked" | ||
} else { | ||
return $0.body | ||
} | ||
} | ||
|
||
self.authorName = comment.map(\.author.name) | ||
self.authorName = comment.map { | ||
if $0.author.isBlocked { | ||
// TODO(MBL-1026): Use translated string once available. | ||
return "Blocked User" | ||
} else { | ||
return $0.author.name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comments here, re: putting this directly in the model. Would it make it simpler at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this belongs in the model. I think it would complicate things (the model would no longer be encodable which would mess with tests & decoding would be more of a pain). Also, whether a user is blocked or not isn't purely a display thing; it also affects functionality, so it's not like we could do no special handling at all. We also can't consistently always hide blocked user info, since we do want to display usernames of blocked users in messages. Plus, at some point, we might want to add an "unblock" option here, in which case the view model will need to know the real user info. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair! |
||
} | ||
} | ||
|
||
let status = comment.map(\.status) | ||
|
||
|
@@ -109,7 +128,9 @@ public final class CommentCellViewModel: | |
let badge = self.commentAndProject.signal | ||
.skipNil() | ||
.map { comment, _ in | ||
comment.author.id == AppEnvironment.current.currentUser?.id.description ? .you : comment.authorBadge | ||
if comment.author.isBlocked { return Comment.AuthorBadge.backer } | ||
return comment.author.id == AppEnvironment.current.currentUser?.id.description ? .you : comment | ||
.authorBadge | ||
} | ||
|
||
self.authorBadge = Signal.merge( | ||
|
@@ -195,7 +216,7 @@ public final class CommentCellViewModel: | |
} | ||
|
||
public let authorBadge: Signal<Comment.AuthorBadge, Never> | ||
public var authorImageURL: Signal<URL, Never> | ||
public var authorImageURL: Signal<URL?, Never> | ||
public let authorName: Signal<String, Never> | ||
public let body: Signal<String, Never> | ||
public let bottomRowStackViewIsHidden: Signal<Bool, Never> | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup here 👍