For #6210: Graying out "Update" for "dev" extensions

[marcelgerber]

Extensions that are in the dev folder can't be updated automatically any more (the "Update" button is grayed out).

This fixes #6210 @peterflynn
German translation @couzteau

Please test this, I couldn't test this as I've got no own extension to test with.

[dangoor]

Assigning myself to look at this.

[dangoor]

This approach makes the template a little harder to read and also seems to lead to the confusing possibility of two different disabled flags and titles being applied in the template.

It seems to me that it would be clearer to have something like this:

         var isInstalledInUserFolder = (entry.installInfo && entry.installInfo.locationType === ExtensionManager.LOCATION_USER);
         context.allowRemove = isInstalledInUserFolder;
         context.allowUpdate = context.showUpdateButton && context.isCompatible && context.isCompatibleLatest && isInstalledInUserFolder;

and then have another variable like context.updateNotAllowedReason or something like that with the appropriate string for the button title.

How does that sound?

[marcelgerber]

Yes, that's a good idea. I'll change it.

[marcelgerber]

@dangoor Just changed. Is this a good approach?
And another question: Should the notifyCount (the green circle with the number of extensions with updates) mention updates that are not allowed?

[dangoor]

@SAplayer Yes, that looks great. One more thing: this code has a lot of tests. It would be good if you could add a test for this condition.

Excellent thought about the notifyCount, I hadn't thought of that. My opinion, and others could certainly disagree on this, is that including extensions that have updates available but where you can't update automatically because it's not installed in a user folder is actually a good thing because it tells people that they may want to update their dev copies.

What do you think?

[peterflynn]

@dangoor I agree too

[marcelgerber]

I tried adding unit tests for so long, here's what I currently have:

it("should show disabled update button for items that are in dev folder and have a compatible update available", function () {
                    mockRegistry = { "mock-extension-2": makeMockExtension([">0.1", ">0.1"]) };
                    mockLoadExtensions(["dev/mock-extension-2"]);
                    setupViewWithMockData(ExtensionManagerViewModel.RegistryViewModel);


                    runs(function () {
                        var $button = $("button.update[data-extension-id=mock-extension-2]", view.$el);
                        expect($button.length).toBe(1);
                        expect($button.prop("disabled")).toBeTruthy();
                    });
                });

But it doesn't work. Can anyone maybe help?

[dangoor]

@SAplayer I'll try to take a look on Monday. Thanks for working on getting a test in as well!

[dangoor]

@SAplayer Your test looks like it's on the right track, but I think I'll need to dig into it a bit later this week. I will try to get your change with a test landed this week though.

[redmunds]

We don't use milestones on pull requests, I removed Sprint 36 milestone.

[dangoor]

@SAplayer Your test was on the right track... I made some changes to how the mocks were created and got it working. I also added the word "folder" to the English string. This is now merged into master. Thanks for the change!