1279319 - Sync notification displaying warning/error when sync completion completed successfully

Status: RESOLVED FIXED

(Firefox for iOS :: Sync, defect)

Description

[Stephan Leroux [:sleroux]]

When navigating to the new History panel -> Synced devices, a sync call is fired but only syncs the tabs/clients. For me this always returns successful but we're displaying the bookmark sync has failed (warning) every time this completes. It seems that because don't have an `asObject` case for SyncStatus when it's .Good [1] the notification root view controller uses the previous error message that was shown [2] which in my case was the bookmarks error. This would cause the error notification to appear more than once while the user navigates the panels.

[1] https://github.com/mozilla/firefox-ios/blob/master/Providers/Profile.swift#L28
[2] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Notifications/NotificationRootViewController.swift#L242.

Comment 1

[Stephan Leroux [:sleroux]]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1907#attch-to-bugzilla

Comment 2

[Emily Toop (:fluffyemily)]

Comment on attachment 8761785 [details]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1907#attch-to-bugzilla

comment left in PR

Comment 3

[Emily Toop (:fluffyemily)]

Comment on attachment 8761785 [details]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1907#attch-to-bugzilla

LGTM

Comment 4

[Stephan Leroux [:sleroux]]

master 5ad5508bf2be32791e544a1da3ef08145b8ef480

Comment 5

[Stephan Leroux [:sleroux]]

v5.x af4f6c6

Comment 6

[Aaron Train [:aaronmt]]

" Sync notification displaying warning/error when sync completion completed successfully"

Late Friday reading this bug summary but why would we should a warning or error on a successful Sync?

Comment 7

[Aaron Train [:aaronmt]]

(In reply to Aaron Train [:aaronmt] from comment #6)
> " Sync notification displaying warning/error when sync completion completed
> successfully"
> 
> Late Friday reading this bug summary but why would we should a warning or
> error on a successful Sync?

Nevermind!