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

[Reyna, David]

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 %}.

> 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.

- 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
> >>
> >
>