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.

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-1025] Update User object with server value for isBlocked #1891

Merged
merged 6 commits into from Nov 30, 2023

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Nov 29, 2023

📲 What

This makes the isBlocked bool use the server value, for the User model (and the Author subset), whether it's fetched using GraphQL or v1. Tests are updated as well.

Note: I also switched isBlocked to a required instead of an optional bool, since that feels easier to use. If it's missing from the server for some reason, we default to false.

👀 See

Jira

✅ Acceptance criteria

  • Comments from a blocked user show as the "Blocked user" placeholder comment (tests Author/graphQL)
  • Project creator's isBlocked corresponds to whether or not the user is actually blocked (tests User/v1)
  • Verify that posting a comment still works as expected (this is one of the mutations that was changed by adding isBlocked to the Author

@ifosli ifosli added the draft label Nov 29, 2023
@ifosli ifosli added this to the release-5.11.0 milestone Nov 29, 2023
@ifosli ifosli self-assigned this Nov 29, 2023
@@ -240,7 +240,7 @@ internal final class MessagesViewModelTests: TestCase {
self.scheduler.advance()

self.messages.assertValueCount(1)
self.replyButtonIsEnabled.assertValues([false, false, true])
self.replyButtonIsEnabled.assertValues([false, true, true])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is because the user template now has the isBlocked explicitly set, so the isBlocked == false check causes true to be emitted instead. (I also changed isBlocked to be required to avoid weirdness like this in the future; optional bools are weird, especially when we expect them to actually exist.)

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment would also be good in the code!

Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Attention: 38 lines in your changes are missing coverage. Please review.

Comparison is base (3279b51) 87.76% compared to head (e54a5d8) 83.70%.

Files Patch % Lines
KsApi/models/lenses/UserLenses.swift 2.63% 37 Missing ⚠️
KsApi/models/graphql/GraphUser.swift 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1891      +/-   ##
==========================================
- Coverage   87.76%   83.70%   -4.06%     
==========================================
  Files         861     1225     +364     
  Lines       77019   111585   +34566     
  Branches    20426    29687    +9261     
==========================================
+ Hits        67592    93402   +25810     
- Misses       8686    17161    +8475     
- Partials      741     1022     +281     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

nice once!

Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

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

👍

@ifosli ifosli merged commit 36e4086 into main Nov 30, 2023
7 checks passed
@ifosli ifosli deleted the fetchIsBlocked branch November 30, 2023 19:29
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.

None yet

3 participants