feat: add code for querying the /teams/notifications endpoint - WPB-24947#4661
feat: add code for querying the /teams/notifications endpoint - WPB-24947#4661caldrian wants to merge 1641 commits into
Conversation
…9-3fd2c79caa49237a3a843dc23b51e1d29b8d847f' into chore/analytics-setup-refactoring-WPB-17530
…/analytics-setup-refactoring-WPB-17530-part2
Co-authored-by: Jullian Mercier <31648126+jullianm@users.noreply.github.com> Co-authored-by: Christoph Aldrian <caldrian@users.noreply.github.com> Co-authored-by: Christoph Aldrian <christoph.aldrian@wire.com>
Co-authored-by: Jullian Mercier <31648126+jullianm@users.noreply.github.com>
…#3221) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Christoph Aldrian <caldrian@users.noreply.github.com>
Co-authored-by: Jullian Mercier <31648126+jullianm@users.noreply.github.com>
…ude-nse-logs-WPB-15114
…ude-nse-logs-WPB-15114
…8-ab7a03e6b6f178cf4372cb7b54baf9f81d1e22ff' into chore/include-nse-logs-WPB-15114
…ter-mock-property-names-WPB-11829
…/cleanup-branches
…ytics-setup-refactoring-WPB-17530-part2
… chore/cleanup-branches
…/cleanup-branches
…e/cleanup-branches
…ering-apps-in-large-teams-WPB-24947
…rge-teams-WPB-24947
netbe
left a comment
netbe
left a comment
There was a problem hiding this comment.
left some comments before approving
| struct TeamNotificationsResponseV5: Decodable, ToAPIModelConvertible { | ||
|
|
||
| let notifications: [TeamNotificationV5] | ||
| let time: UTCTime? |
There was a problem hiding this comment.
it's maybe the servertime, not sure. Something related to calling so we can calculate the age of call events
| .init( | ||
| element: notifications.flatMap { $0.toAPIModel() }, | ||
| hasMore: hasMore ?? false, | ||
| nextStart: notifications.last?.id.transportString() ?? "" |
There was a problem hiding this comment.
please report to backend to know if this is expected.
jullianm
left a comment
jullianm
left a comment
There was a problem hiding this comment.
Looks good, I left one comment about project structure specifically II think we should keep the notifications decoding & responses objects introduced in v5 closer to the TeamsAPIV5 object.
| // along with this program. If not, see http://www.gnu.org/licenses/. | ||
| // | ||
|
|
||
| struct PaginatedTeamNotificationsResponseV5: Decodable, ToAPIModelConvertible { |
There was a problem hiding this comment.
I see this PR introduced two new folders: EventDecoding and Responses at the top level to process the payload from the notifications endpoint however these models are only related to TeamsAPIV5. From a project structure standpoint, I find it confusing as in other API components we keep these models close to the API version its related to.
I would suggest to either move them to TeamsAPIV5 or create a folder named TeamsAPIV5 which contains these models including the TeamsAPIV5.swift file.
There was a problem hiding this comment.
I tried to align the structure with the UpdateEventsAPI code:
I think it's too much code to be put into a single file and creating a directory TeamsAPIV5 is something we haven't done before if I'm not mistaken.
What do you think, can we leave it like this?
|
|
||
| import Foundation | ||
|
|
||
| struct TeamNotificationV5: Decodable { |
There was a problem hiding this comment.
this decodable model should be closer to the API version it was introduced in (TeamsAPIV5) see my other comment.
…ering-apps-in-large-teams-WPB-24947
|
| .init( | ||
| element: notifications.flatMap { $0.toAPIModel() }, | ||
| hasMore: hasMore ?? false, | ||
| nextStart: notifications.last?.id.transportString() ?? "" |
There was a problem hiding this comment.
suggestion: ok maybe worth adding a comment for this in the public header
Issue
This PR adds networking code for the
/teams/notificationsendpoint.It's not yet used, a follow-up PR will finish that.
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: