Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Follow count fix #233

Merged
merged 3 commits into from
Nov 3, 2023
Merged

Follow count fix #233

merged 3 commits into from
Nov 3, 2023

Conversation

JohnOberhauser
Copy link
Member

  • Making the database update your following count when you follow or unfollow someone
  • Adding unit tests for these methods
  • Creating a base test for repository testing

@JohnOberhauser JohnOberhauser requested review from a team, devotaaabel and timc-mozilla and removed request for a team November 2, 2023 20:40
@@ -105,23 +106,44 @@ class AccountRepository internal constructor(
suspend fun getAccountFavourites(): List<Status> =
accountApi.getAccountFavourites().map { it.toExternalModel() }

suspend fun followAccount(accountId: String) {
suspend fun followAccount(
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be open to putting this logic in a use case (or at least a separate function)? My thinking is that we'd only want this to happen if a user is looking at some UI with the follow count. Otherwise it's kind of wasted cycles

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this logic should run even if you're not looking at the UI. You might navigate back to a screen that is affected by this and expect the UI to be updated. I'm worried about someone not calling this function when following an account, and creating a bug in the process.

I actually have a very related topic added to the TIP agenda. Is it okay to merge this in as-is for now? I might end up refactoring a lot of stuff into use-cases depending on the outcome of that conversation.

@JohnOberhauser JohnOberhauser merged commit f186e86 into main Nov 3, 2023
3 checks passed
@JohnOberhauser JohnOberhauser deleted the followCount branch November 3, 2023 16:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants