Refresh Feed + Profile view after successful recovery #912
Conversation
Ensures the feed is populated when dismissing recovery
bfollington
force-pushed
the
2023-09-29-refresh-feed-and-profile
branch
from
9ae5115 to
609d7a2
Compare
| } | ||
| } | ||
|
|
||
|
|
||
| static func detailStackNotification( |
There was a problem hiding this comment.
This seems somewhat surprisingly named to me. AFAICT, it's not sending up a notification, but forwarding down an intercepted action?
It's also somewhat unusual to have an update fn that takes an action from another context.
I think it might be more straightforward to have these update arms inlined, or given separate static methods. I think that might make them easier to test, or to handle behavior differently in future.
There was a problem hiding this comment.
I've inlined the update calls, but I'm not sure that this makes sense to me:
It's also somewhat unusual to have an update fn that takes an action from another context.
It's just extracting common functionality in a way that's almost identical to a cursor's update function, perhaps if it were named forwardWithRefresh it wouldn't seem so odd? Given the footgun opportunity to omit .detailStack() around the forwarded actions I thought one mapping site was safer & equally testable (I made this mistake during the refactor).
I suppose I don't see a reason to repeatedly write the same code 4 times upfront, you can always branch one out if/when it's called for.
There was a problem hiding this comment.
Fair point. If you like, we can leave as-was, but I would prefer a different name, since notification is used elsewhere to mean "sending a message up into another parent context we don't know anything about"
Working on #912 I realised that merging notes was broken. The addresses for merge suggestions were not being relativized when retrieved from SQLite, so they had a `peer` of `.did()` which made `writeMemo` blow up thinking it couldn't write to that path. I've changed `searchRenameSuggestions` to relativize addresses.
|
@gordonbrander I've added tests for
|
Ensures the feed is populated when dismissing recovery mode after a successful recovery. Refreshing the profile is less important, given that the feed is the default tab, but probably still worth refreshing it to be safe.
I found a few other refresh and notification-centric issues too.
Changes
succeedSaveMemo->succeedSaveEntry)