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

Index and purge peers by petname + rework follow / unfollow lifecycle #990

Merged
merged 13 commits into from
Nov 14, 2023

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented Nov 8, 2023

Improves #989
Fixes #946
Fixes #977

This PR remodels the follow/unfollow/rename operations and refresh lifecycle. It also introduces support for indexing multiple peers that point to the same DID.

Screen.Recording.2023-11-13.at.1.04.23.pm.mov

I intend to refactor the "wait for petname resolution" logic and background jobs in general as a follow up in #996

Tasks

  • Fix edgecase with alias rendering
  • Index peers by petname rather than DID
  • Move all follow/unfollow/rename communication to app root
  • Allow refreshing profile while displaying existing loaded content
  • Allow omnibar animation to start and stop multiple times
  • Refresh profile immediately after follow / unfollow
  • Kick off resolution immediately after rename
  • Kick off re-index immediately after follow / unfollow / resolve / rename
  • Trigger toast notifications for follow / unfollow / rename

@bfollington bfollington force-pushed the 2023-11-08-index-peer-by-petname branch from d37f015 to bc771d6 Compare November 13, 2023 03:06
@bfollington bfollington marked this pull request as ready for review November 13, 2023 03:42
@bfollington bfollington changed the title Index and purge peers by petname Index and purge peers by petname + rework follow / unfollow lifecycle Nov 13, 2023
Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

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

LGTM, with a few nits and questions.

Also, given this is a pretty big change, and touches the data layer, are there any additional unit tests we should write for this?

guard self.state == .ready else {
throw DatabaseServiceError.notReady
}
let savepoint = "purge"

guard let peer = try readPeer(petname: petname) else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this guard clause necessary? If we delete a peer, but there are no peers with that name, the end result should be the same.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to know the DID of a peer to delete from the memos table, which we can only get by reading the peer.

@gordonbrander
Copy link
Collaborator

gordonbrander commented Nov 14, 2023

@bfollington one more note: the animations for refreshing profile seem wonky, especially around the feed (seems to animate from the bottom?)

@bfollington bfollington merged commit 4202840 into main Nov 14, 2023
@bfollington bfollington deleted the 2023-11-08-index-peer-by-petname branch November 14, 2023 06:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: 🌱 Done
2 participants