Code review comment for lp:~jcsackett/launchpad/unknown-blueprints-service-597738

I just discovered I missed comments within the diff; sorry for not replying on Friday.

> If the enum is wrong, how will the user get here? The tab will be
> disabled, so we assume they have a direct URL?

I'm not sure the tab will be disabled--if no blueprints existed, and the enum said "EXTERNAL" or something, you could still get to this view so the "${Pillar} does not use blueprints" could be displayed. In this instance if it's set to EXTERNAL but people have been using the blueprints, we continue to display the blueprints information when you get here.

The involvement menu link is disabled, but the tab along the top of the page is still there, and of course nothing stops someone from going to blueprints.launchpad.net/$PILLAR.

Now, I'm open to debate on whether we should just turn off the feature regardless of whether or not blueprints are already being used--I erred on the side of not allowing someone to just delete a bunch of work, but that might not be the decision we want to make here.

>> self.verify_involvment(context)
>
> You didn't introduce it, but please fix the typo of s/verify_involment/verify_involvement/

Uhm, not sure what you mean--looks like it already is verify_involvement?

>> def test_adaptable_to_specificationtarget(self):
>> @@ -87,6 +92,63 @@
>> '<div id="involvement" class="portlet involvement">' in view())
>>
>>
>> +class TestHasSpecificationsTemplates(TestCaseWithFactory):
>> + """Tests the selection of templates based on blueprints usage."""
>> +
>> + layer = DatabaseFunctionalLayer
>> +
>> + def setUp(self):
>> + super(TestHasSpecificationsTemplates, self).setUp()
>> + self.user = self.factory.makePerson()
>> + self.product = self.factory.makeProduct()
>> + self.naked_product = removeSecurityProxy(self.product)
>> + login_person(self.user)
>
> While I'm not a prude, I must ask why all of the nakedness here?
> Couldn't you just make self.user be the owner of the product and
> proceed fully clothed?

I wanted to test what a non-owner user has happen here; that may be the same thing as self.user, but it may not--I've been bitten by using owners/admins in the past.

> Why use 'specifications' just the once? The page says 'blueprints'
> six times and it is unclear that specifications and blueprints are the
> same from the context.
>
> Should we be more frank:
>
> <project> tracks specifications somewhere besides Launchpad, but we
> don't know where. You should check with the project maintainers for
> the location.
>
> I'm just throwing that out for thought. I'm not sure I even like the idea.

I'm playing with this right now related to the UI review; neither blueprints nor specifications are totally right for the copy, per Curtis's comments.

But consider your comments thrown into the mix.

> Thanks for the additional drive-by fixes.

I do what I can. :-P