Closed Bug 883289 Opened 11 years ago Closed 8 years ago

Refresh icon if necessary when updating manifest

Categories

(Marketplace Graveyard :: Developer Pages, defect, P3)

x86_64
Linux
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: mat, Unassigned)

Details

(Whiteboard: [marketplace-transition])

We fetch the icon for the app from the manifest at creation, but then never again. If it was unavailable for some reason at submission, or if it changes later, we never seem to update it. 

We should probably check & fetch icon if necessary when we update the manifest for hosted apps, and when a new version is uploaded for packaged apps. Since a custom icon can be provided by the user, we need to do something to avoid overwriting those and only update icons automatically created from manifest contents.

That's a problem for some apps in production at the moment (see bug 879342)
Blocks a P2 -> set as P2.
Priority: -- → P2
Assignee: nobody → mpillard
The logic was that if a developer uploads a custom icon on the Edit Listing page, we don't want to overwrite it with an updated icon from the manifest. It also was impossible (don't know if it is now) to determine whether the icon had changed because a.) we didn't store the original icon to diff against and b.) we didn't store the URL of the icon to see if it is different.
Yeah, from what I've seen it seems to be still a problem, we don't store original icon info. It seems to me we should do the following:
- Store an hash of the biggest icon from the manifest on the app model
- Also store a boolean defaulting to False indicating whether the user has uploaded a custom icon or not
- When updating, if that boolean is False, compare the hash and overwrite the icons if the biggest icon in the manifest has a different hash than the one stored.

(We could replace the boolean by some condition like "if the hash is empty" but I'm not a big fan)

Sounds good ?
Perhaps it would be better to store user uploaded icons separately from fetched icons. We could always fetch the updated icons and simply override them if the user has uploaded their own.

"Back in the day" we had an idea to add a media/assets page to the devhub where you could manage your icons/previews/image assets in one place. It never got built. Ideally, you could delete icons that you've uploaded and we'd fall back to the icons that you specify in your manifest.
Do the shortterm fix for the e.me bug right now.

For the long term, do we want to allow people to override the icons specified in the manifest in the developer pages?  UX - is this a valid use case?  Security - any concerns with that?  Currently we let people set any icon they want on the marketplace and do nothing with the one in the manifest.
Flags: needinfo?(rforbes)
Flags: needinfo?(msandberg)
Unblock bug 879342 and lower priority since the short term issue is dealt with in bug 87934.
No longer blocks: 879342
Priority: P2 → P3
Flags: needinfo?(msandberg) → needinfo?(asantos)
I'm not sure how big of a problem this use case actually is, I don't think we'll know until we start getting a greater volume of apps submitted. That said, my guess is this will be an edge case kind of scenario. 

I think it would be good if we can track the image(s) we have and have an alert if the a conflict occurs, like you'd see in source control software. Something like "You've updated your app's icon in your manifest, would you like to use this new icon or the one you uploaded on <uploadDate>?"  and show the two images when the conflict happens. Most devs should be familiar with this kind of conflict message, and then we don't have to jump through the hoops of trying to assume what a developer was trying to do and getting it wrong.
Flags: needinfo?(asantos)
(In reply to Tony Santos [:tsmuse] from comment #7)
> I think it would be good if we can track the image(s) we have and have an
> alert if the a conflict occurs, like you'd see in source control software.
> Something like "You've updated your app's icon in your manifest, would you
> like to use this new icon or the one you uploaded on <uploadDate>?"  and
> show the two images when the conflict happens.

We'd need to do this via email, since it's more than likely that we'd detect the changes during our CRON jobs. We could create a UI for it, but then the question is a. where do we put it so that the developer sees it and knows to do something (we don't have notifications anymore) and b. how do we keep this from getting really annoying (if we pull down a copy of the image every night and it's different than the one we have stored, we send an email. but then the developer gets an email every night).
I think email is a great delivery method for this kind of message, maybe also have it pop up on the next log-in to remind them (assuming they're getting an actionable link in the email that they haven't acted on when the log-in next). Is the issue with the potentially generating nightly emails something we could track to make sure it doesn't happen, like we sent an alert and then note it in the system to not send another one unless it goes ignored for a week or something like that?
(In reply to Tony Santos [:tsmuse] from comment #9)
> also have it pop up on the next log-in to remind them (assuming they're
> getting an actionable link in the email that they haven't acted on when the
> log-in next).

We can't do anything on login since Persona can unexpectedly change your logged in state, unfortunately. A person might not "re-log in" for over a month, but then be reauthenticated three times in the same day. Login is a dark place right now.

> Is the issue with the potentially generating nightly emails
> something we could track to make sure it doesn't happen, like we sent an
> alert and then note it in the system to not send another one unless it goes
> ignored for a week or something like that?

There's a surprising amount of work that something like this would require, namely keeping track of icon hashes, keeping track of which authors we've sent the message to (you can have multiple developers for one app), keeping track of how long those messages have gone without being acted on, keeping track of which of those authors have visited the devhub, etc. I'm not entirely sure that a project that large justifies the return in this case. What can we do to simplify the feature?
Assignee: mpillard → nobody
Flags: needinfo?(rforbes)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Whiteboard: [marketplace-transition]
You need to log in before you can comment on or make changes to this bug.