[MBL-1024] Wire Up Block User Mutation #1893
Conversation
Kickstarter-iOS/Features/Comments/Controllers/CommentRepliesViewController.swift
Show resolved
Hide resolved
Kickstarter-iOS/Features/ProjectPage/Controller/ProjectPageViewController.swift
Outdated
Show resolved
Hide resolved
| .switchMap { input in | ||
| AppEnvironment.current.apiService | ||
| .blockUser(input: input) | ||
| .ksr_delay(AppEnvironment.current.apiDelayInterval, on: AppEnvironment.current.scheduler) |
There was a problem hiding this comment.
Just for my own edition, what's up with ksr_delay everywhere when we make network requests?
There was a problem hiding this comment.
On the whole, LGTM. Left a few questions that may or may not need to be addressed.
Will you be adding tests for the view models? I'm fine with that as a follow-on PR but seems like it would be good to double-check that all of the plumbing is working correctly.
|
@amy-at-kickstarter Thanks - yea, I have add tests on my to-do list in the description above. I wanted to jump-start the review while I do those. |
Kickstarter-iOS/Features/Comments/Controllers/CommentsViewController.swift
Outdated
Show resolved
Hide resolved
scottkicks
force-pushed
the
mbl-1024-wire-up-blocking-mutation
branch
from
0e58684
to
0880bce
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1893 +/- ##
==========================================
+ Coverage 83.70% 83.74% +0.03%
==========================================
Files 1225 1226 +1
Lines 111585 111767 +182
Branches 29687 29725 +38
==========================================
+ Hits 93402 93598 +196
+ Misses 17161 17152 -9
+ Partials 1022 1017 -5 ☔ View full report in Codecov by Sentry. |
| @@ -108,6 +111,67 @@ internal final class CommentRepliesViewModelTests: TestCase { | |||
| } | |||
| } | |||
|
|
|||
| func testDidBlockUser_EmitsOnSuccess() { | |||
There was a problem hiding this comment.
Just skimmed but tests LGTM as well.
One thing I'm thinking about in the back of my head - blocking functionality is pretty much identical in the three view models we implement it in. Do we have a pattern for sharing code between view models? (This isn't blocking feedback for this change, but I'm curious if there's a cleaner pattern.)
It's a good callout. We could create a BlockUser service that can be injected and used across the app. I opted to stick with what's more common in the app currently because this is more time-sensitive, but I think it could be cleaner. |
📲 What
Now that the block mutation has been added, let's implement it.
🤔 Why
So we can block users!
🛠 How
We are letting users block someone from the following places:
When we call this mutation via
AppEnvironment.current.apiService.blockUser...either an error will occur or it'll be successful. So, in each view model, I've set up an output for each case that will in turn present a success or error message banner.We currently don't have the error messages that we can receive from the backend translated and localized so for now we are showing a generic "This didn't work" message. Not ideal. Showing a more useful message like, "You can only block someone that doesn't have a live project you have backed" would be better for obvious reasons so I think this should be a fast follow. Added a //TODO for this inline.
👀 See
expects the same UI/UX that we've been working with up to now.
✅ Acceptance criteria
We can block someone through each location listed above.
Attempting to block someone who has already been blocked will return an error
Attempting to block someone without internet will throw an error
The UI reflects newly blocked users
you can also double-check that someone has been blocked successfully here https://staging.kickstarter.com/graphiql
⏰ TODO
idvsuidchange to be made on prod. Once that's updated we can test that the app reloads after blocking and filters out blocked content correctly