432232 - Feature Manager should not allow sandbox'ed items to be chosen

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect)

Description

[Basil Hashem [:baz]]

The Feature Manager (both Admin and Editor) should not allow sandbox'ed add-ons to be selected.

Comment 1

[Justin Scott [:fligtar]]

Not related to 3.5 Dev CP revamp. Wil wrote the feature manager - can you take this early this week or shall I punt it to another milestone?

Comment 2

[Wil Clouser [:clouserw]]

Attachment: only allow public add-ons

I changed the admin and editor feature forms.  The autocomplete won't suggest a feature if it's not public now unless you type in the id for the add-on.  Even if you type in an id and hit submit it still won't accept the feature though.

This code uses wenzel's getAddon() patch from bug 450442, btw.

Comment 3

[Fred Wenzel [:wenzel]]

Comment on attachment 338802 [details] [diff] [review]
only allow public add-ons

The patch looks good so far, however:

+                $_addon = $this->Addon->getAddon($this->data['Addon']['id']);

If you don't specify any "associations", you'll only get id and name back, no status. We could change that (I am not completely sure what the default field set to return should be, just that it should be small), or you make a new association "status", or you use one of the existing ones that contains status.

Comment 4

[Wil Clouser [:clouserw]]

(In reply to comment #3)
> (From update of attachment 338802 [details] [diff] [review])
> The patch looks good so far, however:
> 
> +                $_addon = $this->Addon->getAddon($this->data['Addon']['id']);
> 
> If you don't specify any "associations", you'll only get id and name back, no
> status. We could change that (I am not completely sure what the default field
> set to return should be, just that it should be small), or you make a new
> association "status", or you use one of the existing ones that contains status.

I get a pretty decent amount of data (couple pages in a print_r) with that call and definitely got the status.  You're all the way up to date and you only get two things back?

Comment 5

[Fred Wenzel [:wenzel]]

(In reply to comment #4)
> I get a pretty decent amount of data (couple pages in a print_r) with that call
> and definitely got the status.  You're all the way up to date and you only get
> two things back?

Ouch, that's a bug in getAddons() then. Fixed in r18294. Sorry!

Comment 6

[Wil Clouser [:clouserw]]

(In reply to comment #5)
> (In reply to comment #4)
> > I get a pretty decent amount of data (couple pages in a print_r) with that call
> > and definitely got the status.  You're all the way up to date and you only get
> > two things back?
> 
> Ouch, that's a bug in getAddons() then. Fixed in r18294. Sorry!

That didn't change the results I get back.  It did for you?

Comment 7

[Fred Wenzel [:wenzel]]

(In reply to comment #6)
> That didn't change the results I get back.  It did for you?

Yes, did you svn up?

Array
(
    [Addon] => Array
        (
            [id] => 1865
        )

    [Translation] => Array
        (
            [name] => Array
                (
                    [string] => Adblock Plus
                    [locale] => en-US
                    [textdir] => ltr
                    [locale_html] => 
                )

        )

)



Notice: Undefined index: status in /Users/fred/dev/addons/site/app/controllers/editors_controller.php on line 605

Comment 8

[Fred Wenzel [:wenzel]]

(I needed to run it with JS disabled in order to see the error message, because you can't see them easily when it's an AJAX request).

Comment 9

[Wil Clouser [:clouserw]]

Attachment: update to use new getAddon()

Alright, here is a new patch for use with the new code.  I added status to the default fields.

Comment 10

[Fred Wenzel [:wenzel]]

Comment on attachment 338895 [details] [diff] [review]
update to use new getAddon()

Yes, this wfm.

Comment 11

[Wil Clouser [:clouserw]]

thanks, r18304