[Toaster] V2: [review-request] configure variables page: dreyna/hardcoded_submit_7246_7371

Tue Mar 3 01:46:42 PST 2015

On 02/03/2015 20:23, "Reyna, David" <david.reyna at windriver.com> wrote:

>Hi Belén,
>
>> if you delete all our preset variables and then
>> add back IMAGE_INSTALL_append the tooltip of
>> the delete icon doesn't come up.
>
>Fixed and pushed. Code was in the wrong {% if %}.

Thanks!

>
>> The other one is more of a problem: the validation against variables
>> currently set in the page has stopped working. I was able to add 2
>> distros
>> :/
>
>I cannot replicate this, adding a second "DISTRO" (or any other
>variable). How did you do this?
>
>What I could replicate was for example adding "distro" in addition to
>"DISTRO". This upper/lower case test is now fixed and pushed, in addition
>to dangling error class markers when there were cumulative errors.

agreed: I am not sure what happened yesterday when I tried, but whatever
it was, it's working properly now :)

So from the UI side this is ready to submit upstream.

Thanks!

Belén

>
>- David
>
>
>> -----Original Message-----
>> From: Barros Pena, Belen [mailto:belen.barros.pena at intel.com]
>> Sent: Monday, March 02, 2015 3:37 AM
>> To: Reyna, David
>> Cc: toaster at yoctoproject.org
>> Subject: Re: V2: [Toaster] [review-request] configure variables page:
>> dreyna/hardcoded_submit_7246_7371
>>
>> Hi David,
>>
>> All the issues in my previous email are fixed :) The bad news is that
>> I've
>> found a couple more:
>>
>> One is pretty small: if you delete all our preset variables and then
>> add
>> back IMAGE_INSTALL_append using the 'add variable' form, the tooltip of
>> the delete icon doesn't come up.
>>
>> The other one is more of a problem: the validation against variables
>> currently set in the page has stopped working. I was able to add 2
>> distros
>> :/
>>
>> Thanks!
>>
>> Belén
>>
>> On 28/02/2015 04:45, "Reyna, David" <david.reyna at windriver.com> wrote:
>>
>> >Hi Belén.
>> >
>> >Ok, I have fixed the issues and re-pushed my commit
>> >"dreyna/hardcoded_submit_7246_7371".
>> >
>> >[x] "If you add manually one of our default variables, I can briefly
>> see
>> >an
>> >error alert coming up, then disappearing very quickly."
>> >
>> >This was a race condition between the 'postEditAjaxRequest' and the
>> page
>> >reload. Fixed.
>> >
>> >[x] "When you add back SDKMACHINE, when you click the 'change' icon
>> the
>> >Cancel button is disabled"
>> >[x] "When you add back PACKAGE_CLASESS, the 'Save' button is disabled,
>> so
>> >you
>> >cannot make any changes to the variable value"
>> >
>> >This was really odd and took time to figure out. It turns out that
>> with a
>> >soft reload these two buttons (and only these two buttons) get a
>> >disabled="" attribute inserted magically. The fix was to do a hard
>> reload
>> >instead (which is anyway better in the long run).
>> >
>> >[x] "... I found the message a bit confusing ..."
>> >
>> >I went with the generic: "You cannot set this variable here. Please
>> set
>> >it in the project main page", with a link to that project's main page.
>> >
>> >[x] "The only issue I've found is that the MACHINE variable is showing
>> up
>> >..."
>> >
>> >Filter added for both the blacklist and {MACHINE,BBLAYERS} in the
>> initial
>> >page serve and the XHR serve.
>> >
>> >- David
>> >
>> >> -----Original Message-----
>> >> From: Barros Pena, Belen [mailto:belen.barros.pena at intel.com]
>> >> Sent: Friday, February 27, 2015 5:15 AM
>> >> To: Reyna, David
>> >> Cc: toaster at yoctoproject.org; DAMIAN, ALEXANDRU
>> >> Subject: Re: [Toaster] [review-request] configure variables page:
>> >> dreyna/hardcoded_submit_7246_7371
>> >>
>> >> I just found a couple of other things, sorry:
>> >>
>> >> * If you add manually one of our default variables, I can briefly
>> see
>> >> an
>> >> error alert coming up, then disappearing very quickly.
>> >>
>> >> * When you add back SDKMACHINE, when you click the 'change' icon the
>> >> Cancel button is disabled.
>> >>
>> >> * When you add back PACKAGE_CLASESS, the 'Save' button is disabled,
>> so
>> >> you
>> >> cannot make any changes to the variable value
>> >>
>> >> Cheers
>> >>
>> >> Belén
>> >>
>> >>
>> >>
>> >> On 27/02/2015 12:10, "Barros Pena, Belen"
>> <belen.barros.pena at intel.com>
>> >> wrote:
>> >>
>> >> >Hi David,
>> >> >
>> >> >This is working great. I only have a couple of small comments
>> >> (inline):
>> >> >
>> >> >On 27/02/2015 04:08, "Reyna, David" <david.reyna at windriver.com>
>> wrote:
>> >> >
>> >> >>Hi Belén,
>> >> >>
>> >> >>This commit ³dreyna/hardcoded_submit_7246_7371³ fixes these items:
>> >> >>
>> >> >>  7246 ³Hardcoded variables in /configuration page²
>> >> >
>> >> >The only issue I've found is that the MACHINE variable is showing
>> up
>> >> in
>> >> >the page. I think we should filter it out, particularly because the
>> >> >validation you have added to the 'add variable' form for preventing
>> >> people
>> >> >setting the machine in this page.
>> >> >
>> >> >>  7371 ³Toaster managed mode: configuration variables page allows
>> >> empty
>> >> >>inputs into database²
>> >> >>
>> >> >>
>> >> >>  * Finally, I added/fixed code to enforced the blocking of the
>> >> variables
>> >> >>that are managed outside this page, specifically BBLAYERS and
>> MACHINE
>> >> >>which are handled by the project detail page.
>> >> >
>> >> >This is very nice, I found the message a bit confusing, because it
>> >> gives
>> >> >me a reason but does not tell me what to do. I know this might
>> require
>> >> >some hard coding, but could we have different messages for the
>> MACHINE
>> >> and
>> >> >the BBLAYERS cases?
>> >> >
>> >> >So if you try to set the MACHINE, the message should say:
>> >> >
>> >> >"You cannot set this variable here. Please set the MACHINE in the
>> >> project
>> >> >main page"
>> >> >
>> >> >If you try to set BBLAYERS, the message should say:
>> >> >
>> >> >"You cannot set this variable here. Please add and remove layers in
>> >> the
>> >> >project main page"
>> >> >
>> >> >If this is too much and you rather use a single message, it could
>> say:
>> >> >
>> >> >"You cannot set this variable here. Please set it in the project
>> main
>> >> >page"
>> >> >
>> >> >In all the above messages, "project main page" should be a link to
>> the
>> >> >project page.
>> >> >
>> >> >
>> >> >I hope I am not asking too much.
>> >> >
>> >> >Thanks!!
>> >> >
>> >> >Belén
>> >> >
>> >> >>
>> >> >>- David
>> >> >>
>> >> >>
>> >> >
>> >> >--
>> >> >_______________________________________________
>> >> >toaster mailing list
>> >> >toaster at yoctoproject.org
>> >> >https://lists.yoctoproject.org/listinfo/toaster
>> >>
>> >
>>
>