-
Notifications
You must be signed in to change notification settings - Fork 1
Updated events matching #39
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
base: develop
Are you sure you want to change the base?
Conversation
if matches { | ||
state.activities.sortedInsert(activityData, sorting: state.activitiesSorting) | ||
} else { | ||
state.activities.remove(byId: activityData.id) | ||
} |
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.
e.g. update event changes the data in a way it does not match the query anymore
/// Refetches up to 30 feeds when feed added event is received and local filtering is unavailable. | ||
/// | ||
/// Example is query with member id filter where there can be many members, but fetching all of these | ||
/// is expensive. | ||
private func subscribeToRefetch() { |
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.
There is not any simple solutions here for the case where query contains members key. We need to make some sort of API call. I considered fetching members, but that is probably too expensive if there are many members. So instead, I went for a little bit simplified approach where we just refetch feeds again (max 30). Felt like it gets too complex for refetching all (e.g. someone has 100 feeds paginated). We could improve this later if this is actually needed.
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.
This will have jumps if you are somewhere scrolling, right? Also, when you scroll past those 30 posts, you won't see them anymore.
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.
Currently: if you have 50 loaded, it will refresh first 30 and other 20 are not refreshed. Therefore no jumps.
I could take a step further and refresh them all. Maybe it is not so bad after all
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.
I changed this around, now it does full refetch and updates the data once. Therefore, no jumps. Moreover, I added proper handling of the updated event which might trigger removing it from the feed (e.g. does not match with the query, but can't be matched locally due to missing data)
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.
Now it is magical, list updates properly based on web-socket events. Uses local matching or if not possible, does full refetch after randomly picked 5 second delay.
SDK Size
|
Generated by 🚫 Danger |
Public Interface- @MainActor public class PollListState: ObservableObject
+ @MainActor public final class PollListState: ObservableObject, StateAccessing
- @MainActor public class CommentReactionListState: ObservableObject
+ @MainActor public final class CommentReactionListState: ObservableObject, StateAccessing
- @MainActor public class BookmarkFolderListState: ObservableObject
+ @MainActor public final class BookmarkFolderListState: ObservableObject, StateAccessing
- @MainActor public class ModerationConfigListState: ObservableObject
+ @MainActor public final class ModerationConfigListState: ObservableObject, StateAccessing
- @MainActor public class MemberListState: ObservableObject
+ @MainActor public final class MemberListState: ObservableObject, StateAccessing
- @MainActor public class CommentReplyListState: ObservableObject
+ @MainActor public final class CommentReplyListState: ObservableObject, StateAccessing
- @MainActor public class ActivityReactionListState: ObservableObject
+ @MainActor public final class ActivityReactionListState: ObservableObject, StateAccessing
- @MainActor public class PollVoteListState: ObservableObject
+ @MainActor public final class PollVoteListState: ObservableObject, StateAccessing
- @MainActor public class FeedState: ObservableObject
+ @MainActor public final class FeedState: ObservableObject, StateAccessing
- @MainActor public class BookmarkListState: ObservableObject
+ @MainActor public final class BookmarkListState: ObservableObject, StateAccessing
- @MainActor public class FollowListState: ObservableObject
+ @MainActor public final class FollowListState: ObservableObject, StateAccessing
- @MainActor public class ActivityListState: ObservableObject
+ @MainActor public final class ActivityListState: ObservableObject, StateAccessing
- @MainActor public class ActivityState: ObservableObject
+ @MainActor public final class ActivityState: ObservableObject, StateAccessing
- @MainActor public class CommentListState: ObservableObject
+ @MainActor public final class CommentListState: ObservableObject, StateAccessing
- @MainActor public class ActivityCommentListState: ObservableObject
+ @MainActor public final class ActivityCommentListState: ObservableObject, StateAccessing
- @MainActor public class FeedListState: ObservableObject
+ @MainActor public final class FeedListState: ObservableObject, StateAccessing |
…s everything, MemberList injects feedId, and BookmarkFolderList will get added field
e7d62e1
to
c711bce
Compare
return result.models | ||
} | ||
|
||
/// Refetches up to 30 feeds when feed added event is received and local filtering is unavailable. |
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.
what do you mean by "feed added"? Usually, we won't have many such events.
/// Refetches up to 30 feeds when feed added event is received and local filtering is unavailable. | ||
/// | ||
/// Example is query with member id filter where there can be many members, but fetching all of these | ||
/// is expensive. | ||
private func subscribeToRefetch() { |
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.
This will have jumps if you are somewhere scrolling, right? Also, when you scroll past those 30 posts, you won't see them anymore.
|
🔗 Issue Links
Resolves: IOS-1166
🎯 Goal
📝 Summary
FeedsQuery
hasmembers
andfollowingFeeds
keys which do not have local data (local matching not possible). In this case we'll refetch the feeds list again (long delay for avoiding too many API calls). It has to work magically without having any external filter closures like we have in chatMembersQuery
does not have feed id in the data model, but we can easily inject itBookmarkFoldersQuery
does not have data for userId, but backend will add that (solved later)StateAccessing
protocol for cleaning up common access methods🛠 Implementation
🎨 Showcase
🧪 Manual Testing Notes
☑️ Contributor Checklist
docs-content
repo