-
Notifications
You must be signed in to change notification settings - Fork 25
fix: shared "Drive" tab not displaying conversation names - WPB-23032 #4181
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: release/cycle-4.15
Are you sure you want to change the base?
Conversation
Test Results 2 files 185 suites 2m 6s ⏱️ For more details on these failures, see this check. Results for commit 405c2a4. ♻️ This comment has been updated with latest results. |
WilhelmOks
left a comment
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.
nice work
|
samwyndham
left a comment
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.
Nice work. Left a sanity check comment as I thought I recalled that conversation name might be part of the nodes response. Maybe something to double check with Charles if you haven't already.
|
|
||
| /// The name of the `cell` used for Wire Drive file management. | ||
| var wireCellName: String { get } | ||
| var wireDriveName: String { get } |
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.
thought: I wonder if we should still use the term cell in some cases because this is what it really is? Or maybe node because I guess it is the name of the root node of the conversation. So maybe wireDriveNodeName? What do you think?
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.
Yes you're right, I went autopilot mode here I shouldn't have renamed it. I'll fix that, probably wireDriveNodeName makes sense
| return context.fetchOrAssert(request: request) | ||
| } | ||
|
|
||
| static func fetchDriveConversations(in context: NSManagedObjectContext) -> [ZMConversation] { |
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.
sanity check: Do we definitely need to do this. The conversation name isn't returned as part of the node response somewhere?
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.
No it's not, as far as I remember during the sorting & filtering refinement I asked Charles if we would get the conversation linked to the node in the response and he answered that we would need to get all conversations locally and pass their IDs to the API to filter properly (I think they don't map it on BE side). However, let me double check that because I might be wrong.
Though I think we'd still need that code as we'll need to know the participants that are part of the conversation (out of scope for this ticket but I'm kind of preparing the sorting & filtering feature too).
WPB-23032
Issue
This PR fixe an issue where, in the shared "Drive" tab, we don't display the conversation name the file was shared in.
It also takes this opportunity to do some preparation work for the sorting & filtering feature coming up (filtering by conversations and participants)
Simulator.Screen.Recording.-.Snapshots.-.2026-01-23.at.15.45.49.mov
Testing
go to shared "Drive" tab, the subtitle of the cell should display the file owner and the conversation name it's linked (i.e "userA in conversationB")
conversation files list should remain unchanged
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: