[E-1.1] Add-ons Manager Integration Changes

RESOLVED FIXED in BW-M5

Status

addons.mozilla.org Graveyard
Collections
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: kinger, Assigned: mackers)

Tracking

unspecified
BW-M5
Dependency tree / graph

Details

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
Auto-publisher collections are visually distinct from other collections in the Subscriptions pane.

Is this designed yet?
Morphing this bug to reflect all the UI updates to the existing Add-ons Manager Integration.
Assignee: fligtar → brian
Hardware: x86 → All
Summary: [E-1.6.2] Subscriptions Pane → [E-1.1] Add-ons Manager Integration Changes
Created attachment 365252 [details]
Add-ons Manager Integration mockup, v1

This is a mockup of how the various states of the Add-ons Manager Subscriptions pane should look.
Created attachment 365253 [details]
Install Button mockup, v1

This is a mockup of how the extension install button should look.
(Reporter)

Comment 4

9 years ago
Based on Add-ons Manager Integration mockup, v1, here are the changes and missing features I see needed from the current implementation:

1) View Site button becomes a View Collection link
2) Remove button becomes Unsubscribe link
3) Update button needs to be removed
4) In right panel, the header needs to be expanded to include an icon and a description if the subscription
5) Publisher Comments integration
6) Special treatment for auto-publisher subscriptions at the top of the listing in the left panel.
(Reporter)

Updated

9 years ago
Depends on: 481409
(Reporter)

Comment 5

9 years ago
(In reply to comment #4)
> 1) View Site button becomes a View Collection link
> 2) Remove button becomes Unsubscribe link
> 3) Update button needs to be removed

Done. I've filed bug 481409 to handle state and action if these links.
Yes, that list of changes looks about right. I would also add that I removed the "Category" from underneath the "Added" part, and instead we display who published the add-on to the collection when expanded.

I'm not sure if we had the "no subscriptions" state coded already, but we now have text and layout for that as well (before it was described as only being in the right panel).
(Reporter)

Comment 7

9 years ago
(In reply to comment #6)
> I'm not sure if we had the "no subscriptions" state coded already, but we now
> have text and layout for that as well (before it was described as only being in
> the right panel).

I broke out bug 481628 for that.
Depends on: 481628
(Assignee)

Comment 8

9 years ago
A quick review of the elements of this bug.

1) View Site button becomes a View Collection link

The button is there, but the view collection link goes nowhere. I imagine the link should go to http://%AMO_HOST%/%LOCALE%/firefox/collections/view/%COLLECTION_ID%

However, we can't programatically create this URL, as the API instructs us not to create URLs based on a collection id, but instead to follow hrefs as provided by the REST API.

This href is not available in the API.

2) Remove button becomes Unsubscribe link

As above. The button is there, but we can't construct the URL.

3) Update button needs to be removed

Done.

4) In right panel, the header needs to be expanded to include an icon and a
description if the subscription

The description is now there. I don't see an icon url anywhere.

5) Publisher Comments integration

This is done, but is displaying "Unknown" as the comment author due to missing information in the API.

6) Special treatment for auto-publisher subscriptions at the top of the listing in the left panel.

TBD.
Hi Les, can you take a look at comment #8?
The IDs are changing to UUIDs, so it still good not to try programmatically creating URLs.  What, exactly, are the missing pieces of information?  Is this the list:

* URL to view collection in browser
* URL to unsubscribe from collection in browser
* Name of person adding an addon to a collection

The third thing is actually supported by the API, but I think the scaffolding isn't adding it.

Anything else?
Depends on: 481241
Created attachment 366961 [details] [diff] [review]
patch to include user_id when adding addon to collection in scaffolding, as well as additional links in collections in API

Okay, have a patch here that causes the scaffolding to start annotating addons with who added it to a collection, and some tweaks to the API to include view/subscribe/unsubscribe links suitable for visitation in a browser.

Need a reviewer since rdoherty's out, alas.

Also, the XML produced will look like this (a <links> element added to <collection> elements):

<?xml version="1.0" encoding="utf-8" ?>
<sharing xmlns="http://addons.mozilla.org/" xml:base="http://dev-bandwagon.addons.mozilla.org/en-US/firefox/api/1.3/sharing/">
    <email href="email" />
    <collections href="collections/">
        
        <collection 
            href="collections/b0144e6f-b31d-25f0-d91c-b4b356e23940/" 
            name="floof"
            description="flooz"
            creator="Sancus"
            listed="yes" writable="yes" subscribed="yes" 
            lastmodified="2009-03-11T18:15:28-07:00">
                <links xml:base="http://dev-bandwagon.addons.mozilla.org/en-US/firefox/">
                    <link id="view" href="collections/view/b0144e6f-b31d-25f0-d91c-b4b356e23940" />
                    <link id="subscribe" href="collections/subscribe/b0144e6f-b31d-25f0-d91c-b4b356e23940" />
                    <link id="unsubscribe" href="collections/unsubscribe/b0144e6f-b31d-25f0-d91c-b4b356e23940" />
                </links>
                <addons href="collections/b0144e6f-b31d-25f0-d91c-b4b356e23940/addons/" />
        </collection>
        
    </collections>
</sharing>
Attachment #366961 - Flags: review?(clouserw)
Comment on attachment 366961 [details] [diff] [review]
patch to include user_id when adding addon to collection in scaffolding, as well as additional links in collections in API

tossing an r? to clouserw, since rdoherty's out.  hopefully the addons bandwagon branch doesn't give too much trouble
Attachment #366961 - Flags: review?(clouserw) → review?(rdoherty)
Attachment #366961 - Flags: review?(rdoherty) → review+
Comment on attachment 366961 [details] [diff] [review]
patch to include user_id when adding addon to collection in scaffolding, as well as additional links in collections in API

Tested, looks good and works.
checked the patch in as r23587
(Reporter)

Comment 15

9 years ago
I think this has broken:
https://bandwagon.stage.mozilla.com/en-US/firefox/collections

When trying to subscribe to a collection there, I get:

Missing argument: collection_id

Feed subscriptions are behaving oddly in the extension as well, but that is probably just our code needing to play catchup with these fixes.
(Reporter)

Updated

9 years ago
Assignee: brian → dave
Hmm, I'm guessing that the database changes that go along with the patch have not been applied by IT.  I'm not sure who's doing that for bandwagon... do we have a bug for that, or a regular process for that?

https://wiki.mozilla.org/Update:Developers/Database_Changes
(Assignee)

Comment 17

9 years ago
FWIW, currently the API is broken.

The service document gives my list of collections as normal, but they are without hrefs:

<collection href="collections//" ...

This means the extension will not display any feeds. 

It also means I'm stuck until this is fixed :)
Probably the same issue with pending DB changes :/
(In reply to comment #16)
> Hmm, I'm guessing that the database changes that go along with the patch have
> not been applied by IT.  I'm not sure who's doing that for bandwagon... do we
> have a bug for that, or a regular process for that?
>

We've just been filing IT bugs for it. If the changes are safe to be run in production as well, please specify that they should be run in production and on the bandwagon staging db.
FWIW, bug 484842 is tracking the DB changes
bug 484842 appears closed, and things seem to be working in the API and on the collections web page.  But, they seem to be pretty aggressively cached by the frontends, so requests may return stale broken content for awhile.  

You can forcibly defeat the caching by adding a unique value to a URL, like ?__=12342134 where the number could be the current time
(Reporter)

Comment 22

9 years ago
(In reply to comment #21)
> bug 484842 appears closed, and things seem to be working in the API and on the
> collections web page.  But, they seem to be pretty aggressively cached by the
> frontends, so requests may return stale broken content for awhile.  

Yes, we noticed this before and alerted Justin about it. On production we'll probably need this turned off. I'm not sure what the cache story will be when we go live.

> You can forcibly defeat the caching by adding a unique value to a URL, like
> ?__=12342134 where the number could be the current time

Thanks for the tip.
(Assignee)

Comment 23

9 years ago
(In reply to comment #21)
> You can forcibly defeat the caching by adding a unique value to a URL, like
> ?__=12342134 where the number could be the current time

I have added this functionality to the extension.

If the BANDWAGON_RPC_ENABLE_CACHE_BUSTER constant in rpc/constants.js is positive, the above cache defeating method will take effect.

Revision 23712.

It is currently enabled.
(Assignee)

Comment 24

9 years ago
What's outstanding:

> 4) In right panel, the header needs to be expanded to include an icon and a
> description if the subscription
> 5) Publisher Comments integration

Still waiting on these (bug 470245).

> 6) Special treatment for auto-publisher subscriptions at the top of the listing
in the left panel.

Done.
Depends on: 470245
(Assignee)

Comment 25

9 years ago
I think everything here is complete.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
We're still missing the publisher "by" line under the date added. We replaced the "Category" part with that, and only when the add-on is selected (see mockup). I also noticed that the category text is still in the l10n file.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I added bandwagon.addon.addedby so that l10n will have this new string regardless of when this is implemented. I left the category string in so nothing would break, but it can be deleted when category is removed.
Moving to revised M5 milestone (4/28).
Target Milestone: BW-M3 → BW-M5
(Assignee)

Comment 29

9 years ago
(In reply to comment #26)
> We're still missing the publisher "by" line under the date added. 

I have added the byline. I also made some small aesthetic changes to the addon display to bring it in line with the mockup.

r24532.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.