All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Reyna, David" <david.reyna@windriver.com>
To: "Barros Pena, Belen" <belen.barros.pena@intel.com>,
	"Damian, Alexandru" <alexandru.damian@intel.com>
Cc: Paul Eggleton <paul.eggleton@linux.intel.com>,
	"toaster@yoctoproject.org" <toaster@yoctoproject.org>
Subject: Re: V4: review request for configure details page
Date: Fri, 28 Feb 2014 12:29:41 +0000	[thread overview]
Message-ID: <5E53D14CE4667A45B9A06760DE5D13D055DD79DE@ALA-MBB.corp.ad.wrs.com> (raw)
In-Reply-To: <CF3625DB.3E63B%belen.barros.pena@intel.com>

Hi Belen,

Interesting. I will consider your comments. I understand what you are saying, but never the less it surprised me when I naively used the interface before me because it took my selected filter away from me as it were.

I will note that even without my patch you will not get the behavior you specified.

That is because the "Configuration" page, unlike the other pages, has a default filter that is asserted whenever one of the required parameters are missing.

That is normally only when the page is first visited. But it is also when a search is asserted (due to your desire to clear any current filter), which in this instance will always and immediately assert that default filter instead, in contrast to your specifications. 

That is why I think I see an error state.

- David

> -----Original Message-----
> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> Sent: Friday, February 28, 2014 4:20 AM
> To: Reyna, David; Damian, Alexandru
> Cc: Paul Eggleton; toaster@yoctoproject.org
> Subject: Re: V4: review request for configure details page
> 
> 
> On 28/02/2014 11:33, "Reyna, David" <david.reyna@windriver.com> wrote:
> 
> >> So in the last action 3), when I search, the search should search all
> >> variables stored in the database for the selected build, and when the
> >> search results come in, there should be no filter applied.
> >
> >Hmm. The ability to assert both a filter AND a search was actually a
> >strong selling point when I did my presentation to our Wind River
> >engineers.
> 
> You can do this with the current behaviour. The scope of a filter is the
> content of the table at any given time, which means you can refine the
> results of a search query by applying a filter. Think of it as a
> hierarchy: search rules, filters and sorting are subjected to search. This
> matches what the design communicates: the search is above the filters and
> sorting, encompassing them somehow.
> 
> From the design spec:
> 
> 						The scope of the filters is the content
> currently on the table (this
> means all table pages, not only the one displayed).
> 
> 						The scope of the search is always the
> content of the database.
> 
> 						So:
> 
> 						- if I run a search query, any filter
> applied afterwards will filter
> the content returned by the search query.
> 
> 						- if I run a search query while a
> filter is applied, the filter is
> cleared by the results of the search query (i.e. we display the results of
> the search query and clear the filter applied beforehand). The same
> happens if I click the "Clear search" icon when a filter is applied to a
> set of search results (both search results and applied filter are cleared,
> and the table shows all the builds).
> 
> 
> 
> 
> 
> 
> I hope this makes sense.
> 
> 
> >
> >The use case is this:
> >
> >1) Select the "All Builds" page
> >
> >2) Set the "Completed on" filter do a specific day or time range.
> >
> >3) Set the "Search" to "atom-pc"
> 
> You can still do this, but you would need to search for atom-pc first,
> then use the Completed on filter.
> 
> >
> >In this example, the build team can examine and correlate all builds for
> >a given "Machine" for a given time range (for example for a specific
> >commit window), in order to compare success rates and build times. The
> >engineers in fact wanted more correlation and grouping views and tools of
> >this nature, especially since Toaster already had all the data.
> 
> Yes: more advanced data querying and manipulation mechanism will probable
> be needed at some. I have tried to start simple on this, to create a base
> we can build upon.
> >
> >I have collected several other such user stories, which I am validated
> >and will pass back to the Toaster team soon.
> 
> Cool! Waiting for those
> 
> >
> >I propose that we keep this feature, and sell the heck out of it.
> 
> Whatever we decide, it needs to be consistently applied across the
> interface.
> 
> >
> >- David
> >
> >> -----Original Message-----
> >> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> >> Sent: Friday, February 28, 2014 3:22 AM
> >> To: Reyna, David; Damian, Alexandru
> >> Cc: Paul Eggleton; toaster@yoctoproject.org
> >> Subject: Re: V4: review request for configure details page
> >>
> >>
> >>
> >> On 28/02/2014 11:12, "Reyna, David" <david.reyna@windriver.com> wrote:
> >>
> >> >> I cannot see this working. I am not sure the fix is in your dev
> >>branch.
> >> >
> >> >Hmm, I see the patch in "dreyna/configure-detail-view", in the file
> >> >"basetable_top.html".
> >> >
> >> >Let me describe my test procedure in detail, in case that will reveal
> >> >something.
> >> >
> >> >1) Start the "Configure" page fresh, and click "Bitbake variables".
> >> >
> >> >Observe that the column filter is by default set to the "Description"
> >> >column "Variables with description".
> >> >
> >> >2) Reset the filter to the "Set in file" column, and choose for example
> >> >"Machine configuration variables".
> >> >
> >> >Observe that the new column filter is asserted, and that the
> >> >"Description" filter is off.
> >> >
> >> >3) Put the string "conf/bitbake" in the "Search" text box, and click
> >> >"Search".
> >> >
> >> >Before this fix, the default "Description" filter would suddenly be
> >> >re-asserted, and the user selected "Set in file" filter would be turned
> >> >off.
> >>
> >> Ah, right, the problem is that the above does not match how search
> >>behaves
> >> everywhere else in Toaster (that is why for me "it wasn't working"). The
> >> scope of the search is the full content of the database, so searching
> >> clears your filters.
> >>
> >> So in the last action 3), when I search, the search should search all
> >> variables stored in the database for the selected build, and when the
> >> search results come in, there should be no filter applied.
> >>
> >> >
> >> >With this fix, the filter remains as the "Set in file" filter,
> >>preserving
> >> >the user's selection.
> >> >
> >> >
> >> >What do you observe?
> >> >
> >> >- David
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> >> >> Sent: Friday, February 28, 2014 2:46 AM
> >> >> To: Reyna, David; Damian, Alexandru
> >> >> Cc: Paul Eggleton; toaster@yoctoproject.org
> >> >> Subject: Re: V4: review request for configure details page
> >> >>
> >> >> On 28/02/2014 06:40, "Reyna, David" <david.reyna@windriver.com>
> >>wrote:
> >> >>
> >> >> >Hi again,
> >> >> >
> >> >> >>  * When you cancel the search string, the column filters are also
> >> >>reset.
> >> >> >
> >> >> >Fixed and added to "dreyna/configure-detail-view"!
> >> >>
> >> >> I cannot see this working. I am not sure the fix is in your dev
> >>branch.
> >> >>
> >> >> Cheers
> >> >>
> >> >> Belén
> >> >>
> >> >> >
> >> >> >It turns out that the two search buttons were not passing the
> >>"filter"
> >> >> >and "count" values, and that has the unfortunate side effect of
> >>forcing
> >> >> >every view to execute "_redirect_parameters", which in turn would
> >> >>always
> >> >> >immediately reset any user-selected filters (and "count").
> >> >> >
> >> >> >Here is the simple (and global) fix:
> >> >> >
> >> >> >vvvvvvvvvvvvvvv
> >> >> >
> >> >> >diff --git
> >> >>a/bitbake/lib/toaster/toastergui/templates/basetable_top.html
> >> >> >b/bitbake/lib/toaster/toastergui/templates/basetable_top.html
> >> >> >index 9a8bacb..b19c936 100644
> >> >> >--- a/bitbake/lib/toaster/toastergui/templates/basetable_top.html
> >> >> >+++ b/bitbake/lib/toaster/toastergui/templates/basetable_top.html
> >> >> >@@ -35,6 +35,8 @@
> >> >> >    </div>
> >> >> >    <input type="hidden" name="orderby"
> >> >>value="{{request.GET.orderby}}">
> >> >> >    <input type="hidden" name="page" value="1">
> >> >> >+   <input type="hidden" name="filter"
> >>value="{{request.GET.filter}}">
> >> >> >+   <input type="hidden" name="count" value="{{request.GET.count}}">
> >> >> >    <input class="btn" type="submit" value="Search"/>
> >> >> >    </form>
> >> >> >  <div class="pull-right">
> >> >> >^^^^^^^^^^^^^^^^^
> >> >> >
> >> >> >The other remaining issue (sorting the file column) will need to
> >>wait
> >> >>for
> >> >> >a database fix when Alex gets back.
> >> >> >
> >> >> >- David
> >> >> >
> >> >> >> -----Original Message-----
> >> >> >> From: toaster-bounces@yoctoproject.org [mailto:toaster-
> >> >> >> bounces@yoctoproject.org] On Behalf Of Reyna, David
> >> >> >> Sent: Thursday, February 27, 2014 5:08 PM
> >> >> >> To: Barros Pena, Belen; Damian, Alexandru
> >> >> >> Cc: Paul Eggleton; toaster@yoctoproject.org
> >> >> >> Subject: [Toaster] V4: review request for configure details page
> >> >> >>
> >> >> >> Hi Belen,
> >> >> >>
> >> >> >> On to V4 for "dreyna/configure-detail-view"!
> >> >> >>
> >> >> >> FIXES:
> >> >> >>
> >> >> >> * The description filter tooltip says: "Showing only Show only
> >> >> >>variables..."
> >> >> >> * I see: FILTER:conf/<filter_applied>
> >> >> >> * variable values over 150 characters are not truncated on the
> >>table.
> >> >> >> * Extended variable name patterns should link to the root variable
> >> >>entry
> >> >> >>
> >> >> >> All done!
> >> >> >>
> >> >> >> * When using the search, each row will appear multiple times.
> >> >> >>
> >> >> >> Done! I realized that the "searches" were in fact working inside
> >>the
> >> >> >>vhistory
> >> >> >> list (it is column sorts that are broken). The unexpected
> >>consequence
> >> >> >>is that
> >> >> >> each hit added the record to the queryset, with the observed
> >>anomaly
> >> >> >>that
> >> >> >> multiple file name hits added duplicate rows in the table. Here
> >>is my
> >> >> >> resolution.
> >> >> >>
> >> >> >>     # remove duplicate records from multiple search hits in the
> >> >> >> VariableHistory table
> >> >> >>     queryset = queryset.distinct()
> >> >> >>
> >> >> >> * Variables with no value and no file should be excluded
> >> >> >>
> >> >> >> Done! My tests show this working as expected, where I do see rows
> >> >>with
> >> >> >>a file
> >> >> >> but no value, but I do not see rows without both. Plus, since
> >>this is
> >> >> >>done in
> >> >> >> the view and not in the template, the pagination works as
> >>expected.
> >> >> >>Here is
> >> >> >> my resolution.
> >> >> >>
> >> >> >>     # remove records where the value is empty AND there are no
> >> >>history
> >> >> >>files
> >> >> >>     queryset =
> >> >> >>
> >>queryset.exclude(variable_value='',vhistory__file_name__isnull=True)
> >> >> >>
> >> >> >> I had to play around with this to get the second clause to work,
> >>and
> >> >>I
> >> >> >>would
> >> >> >> like Alex to review it, but as I said it seems to work.
> >> >> >>
> >> >> >>
> >> >> >> PENDING ISSUES:
> >> >> >>
> >> >> >> > Where should I map the help for variable name with patterns of
> >>the
> >> >> >>form
> >> >> >> "do_*"
> >> >> >>
> >> >> >> I sent email on this question separately.
> >> >> >>
> >> >> >> > "My only doubts are about case 2, where
> >> >> >> > I am not sure it makes sense to show the last file when the
> >>search
> >> >> >>string
> >> >> >> > is found in any other file for a certain variable."
> >> >> >>
> >> >> >> I propose that most user searches are for variable names, so the
> >>file
> >> >> >>column
> >> >> >> will appear to reflect the default behavior. If I do not by
> >>default
> >> >> >>include
> >> >> >> the last file, then searching for variable names will _always_
> >>result
> >> >> >>in the
> >> >> >> file column becoming empty (expect by coincidence), which does not
> >> >>look
> >> >> >>like
> >> >> >> the normal behavior for the page.
> >> >> >>
> >> >> >> It may only look odd if you do a search for example for
> >> >>"conf/bitbake",
> >> >> >>where
> >> >> >> you would get all of the matches _plus_ the last file. Given the
> >> >> >>downside in
> >> >> >> the previous and more common use case, I think that this little
> >>bit
> >> >>of
> >> >> >>extra
> >> >> >> information is worth it.
> >> >> >>
> >> >> >> I think a match between both a variable and a file name is
> >>unlikely.
> >> >> >>
> >> >> >> >  * The column sort for files will only test the first file in
> >>any
> >> >> >>vhistory
> >> >> >> list.
> >> >> >> >  * When you cancel the search string, the column filters are
> >>also
> >> >> >>reset.
> >> >> >>
> >> >> >> Still pending.
> >> >> >>
> >> >> >> > Any scripting inside a template should go at the very end of the
> >> >> >> > markup, but before the closing </body> tag.
> >> >> >>
> >> >> >> All I am saying is that unless we define an additional block for
> >> >> >>template-
> >> >> >> specific scripts (and I think that at some point this would be a
> >>good
> >> >> >>idea),
> >> >> >> both the "<head>...</head>" and the "<body>...</body>" are locked
> >> >>inside
> >> >> >> "main.html" and are inaccessible to template files, and thus
> >> >> >>inaccessible for
> >> >> >> template-specific scripts.
> >> >> >>
> >> >> >> Thanks!
> >> >> >> David
> >> >> >>
> >> >> >> > -----Original Message-----
> >> >> >> > From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> >> >> >> > Sent: Thursday, February 27, 2014 4:05 AM
> >> >> >> > To: Reyna, David; Damian, Alexandru
> >> >> >> > Cc: toaster@yoctoproject.org; Paul Eggleton
> >> >> >> > Subject: Re: V3: review request for configure details page
> >> >> >> >
> >> >> >> > Hi David,
> >> >> >> >
> >> >> >> > Thanks for all the good work. I have commented inline on your
> >>email
> >> >> >>when
> >> >> >> > required.
> >> >> >> >
> >> >> >> > There are also a couple of small issues I am still seeing:
> >> >> >> >
> >> >> >> > * The description filter tooltip says: "Showing only Show only
> >> >> >>variables
> >> >> >> > with description". It should say "Showing only variables with
> >> >> >> > description". If needed, we can change the radio button label to
> >> >>say
> >> >> >>just
> >> >> >> > "Variables with description²
> >> >> >> >
> >> >> >> > * Variables with no value and no file are only there because
> >>there
> >> >>is
> >> >> >>a
> >> >> >> > description set for them in documentation.conf. They should be
> >> >> >>excluded
> >> >> >> > from the view (table, search, sorting and filtering).
> >> >> >> >
> >> >> >> >
> >> >> >> > Cheers
> >> >> >> >
> >> >> >> > Belén
> >> >> >> >
> >> >> >> > On 27/02/2014 03:53, "Reyna, David" <david.reyna@windriver.com>
> >> >>wrote:
> >> >> >> >
> >> >> >> > >Hi Belen,
> >> >> >> > >
> >> >> >> > >I have implemented the proposed design changes to the
> >>"Configure"
> >> >> >>page.
> >> >> >> > >
> >> >> >> > >   dreyna/configure-detail-view
> >> >> >> > >
> >> >> >> > >I did need to make some small changes to the proposed design,
> >>as
> >> >> >> > >documented below. There are also some outstanding issues,
> >>which I
> >> >> >>also
> >> >> >> > >list below.
> >> >> >> > >
> >> >> >> > >IMPLEMENTATION:
> >> >> >> > >
> >> >> >> > >> 1) "Showing history for all variables"
> >> >> >> > >
> >> >> >> > >Done.
> >> >> >> > >
> >> >> >> > >> 2) "Searching and filtering" (Hidden file problem)
> >> >> >> > >
> >> >> >> > >Done.
> >> >> >> > >
> >> >> >> > >I did however realize that "search" applies to all fields, not
> >> >>just
> >> >> >>the
> >> >> >> > >file list. This is an issue when you for example search the
> >>rpwsa
> >> >>for
> >> >> >> > >"BBINCLUDED", because suddenly all of the files would disappear
> >> >> >>because
> >> >> >> > >none would match. I do not think that this is what the user
> >>would
> >> >> >>expect
> >> >> >> > >when they are searching for variables and not file names.
> >> >> >> > >
> >> >> >> > >Here is therefore how I implemented the file filtering:
> >> >> >> > >
> >> >> >> > >  'search' 'file filter' Action
> >> >> >> > >  -------- ------------- ----------------------
> >> >> >> > >    -        -           show last file
> >> >> >> > >  value      -           show last file, plus any file names
> >>that
> >> >> >>match
> >> >> >> > >search
> >> >> >> > >    -      value         show only file names that match filter
> >> >> >> > >  value    value         show any file names that match either
> >> >>string
> >> >> >> > >
> >> >> >> > >I think that this meets out needs, and is a "least surprise"
> >>for
> >> >> >>users.
> >> >> >> >
> >> >> >> > In general, this makes sense to me. My only doubts are about
> >>case
> >> >>2,
> >> >> >>where
> >> >> >> > I am not sure it makes sense to show the last file when the
> >>search
> >> >> >>string
> >> >> >> > is found in any other file for a certain variable.
> >> >> >> >
> >> >> >> > Also, when I apply a 'set in file' filter, I see the following
> >> >>string
> >> >> >>on
> >> >> >> > each cell: FILTER:conf/<filter_applied>
> >> >> >> >
> >> >> >> > >
> >> >> >> > >> 3) "Dealing with long values"
> >> >> >> > >
> >> >> >> > >Done. To see it work, turn off the "Description" filter and
> >>look
> >> >>at
> >> >> >>the
> >> >> >> > >variable "BBINCLUDED", which is guaranteed to be bigger than
> >>570
> >> >> >> > >characters.
> >> >> >> >
> >> >> >> > This is working in the history modal dialogs, but variable
> >>values
> >> >> >>over 150
> >> >> >> > characters are not truncated on the table. It's important we do
> >> >>this
> >> >> >>in
> >> >> >> > order to keep the table readable.
> >> >> >> >
> >> >> >> > >
> >> >> >> > >> 4) "What if a variable value is an empty string?"
> >> >> >> > >
> >> >> >> > >Done.
> >> >> >> > >
> >> >> >> > >> 5) "What do we do with the 613 B_* variables?"
> >> >> >> > >
> >> >> >> > >They are now gone from the view.
> >> >> >> > >
> >> >> >> > >> 6) "Linking variables to the Yocto Project reference manual"
> >> >> >> > >
> >> >> >> > >Done.
> >> >> >> > >
> >> >> >> > >However, the statement "we do know that all variables with a
> >> >> >>description
> >> >> >> > >in documentation.conf have an entry in
> >> >> >> > >the Yocto Project reference manual" is not universally true.
> >>Here
> >> >>are
> >> >> >> > >some counter examples:
> >> >> >> > >
> >> >> >> > >  ALLOW_EMPTY_${PN}-dbg
> >> >> >> > >  ASSUME_PROVIDED
> >> >> >> > >  ASSUME_SHLIBS
> >> >> >> > >  BBFILE_PATTERN_core
> >> >> >> > >  BBFILE_PRIORITY_core
> >> >> >> > >  ...
> >> >> >> > >
> >> >> >> > >Some of these are obviously extended patterns, but the code
> >>does
> >> >>not
> >> >> >>know
> >> >> >> > >that, and there is no obvious way to filter for that.
> >> >> >> >
> >> >> >> > Extended patterns should link to the root variable entry (so
> >> >> >> > ALLOW_EMPTY_${PN}-dbg links to the ALLOW_EMPTY entry). But you
> >>are
> >> >> >>right:
> >> >> >> > ASSUME_PROVIDED does not have an entry in the manual. I'll
> >>review
> >> >>the
> >> >> >> > content of documentation.conf to eliminate any mismatches.
> >> >> >> >
> >> >> >> > >
> >> >> >> > >
> >> >> >> > >LIMITATIONS:
> >> >> >> > >
> >> >> >> > >7) There are still the observed defects. I will investigate,
> >>but
> >> >> >>they are
> >> >> >> > >outside of the code under review.
> >> >> >> > >
> >> >> >> > >  * When you cancel the search string, the column filters are
> >>also
> >> >> >>reset.
> >> >> >> > >  * The search will only test the first file in any vhistory
> >>list.
> >> >> >> > >  * When using the search, each row will appear multiple times.
> >> >> >> > >
> >> >> >> > >8) I have added the new Javascript code to support the value
> >> >>extender
> >> >> >> > >dialog to the file "templates/base.html", and it works!
> >> >> >> >
> >> >> >> > It does work wonderfully :)
> >> >> >> >
> >> >> >> > >
> >> >> >> > >
> >> >> >> > >I do not know if this the best place. Nor do I know if it will
> >> >>affect
> >> >> >> > >other pages that would not otherwise use it.
> >> >> >> > >
> >> >> >> > >I do observe that there is currently no way for a page to add
> >> >>local
> >> >> >> > >script code to the "<head>" section, which is currently locked
> >> >>inside
> >> >> >> > >"base.html". Maybe you can add script code to other sections,
> >>but
> >> >>I
> >> >> >>know
> >> >> >> > >that in testing that this new script section would not work
> >>when
> >> >> >>placed
> >> >> >> > >anywhere within "templates/configvars.html".
> >> >> >> >
> >> >> >> > Any scripting that will be reused by several templates should
> >>go on
> >> >> >> > main.js. Any scripting inside a template should go at the very
> >>end
> >> >>of
> >> >> >>the
> >> >> >> > markup, but before the closing </body> tag. That way the page
> >>loads
> >> >> >>first
> >> >> >> > all markup and we show something to users, while the scripts
> >>take
> >> >> >>their
> >> >> >> > time to load. But I think in general we should aim to have all
> >>js
> >> >> >>neatly
> >> >> >> > put away in main.js
> >> >> >> >
> >> >> >> > >
> >> >> >> > >Thanks,
> >> >> >> > >David
> >> >> >> > >
> >> >> >> >
> >> >> >>
> >> >> >> _______________________________________________
> >> >> >> toaster mailing list
> >> >> >> toaster@yoctoproject.org
> >> >> >> https://lists.yoctoproject.org/listinfo/toaster
> >> >>
> >> >
> >>
> >
> 



  reply	other threads:[~2014-02-28 12:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28  1:08 V4: review request for configure details page Reyna, David
2014-02-28  6:40 ` Reyna, David
2014-02-28 10:46   ` Barros Pena, Belen
2014-02-28 11:12     ` Reyna, David
2014-02-28 11:21       ` Barros Pena, Belen
2014-02-28 11:33         ` Reyna, David
2014-02-28 12:19           ` Barros Pena, Belen
2014-02-28 12:29             ` Reyna, David [this message]
2014-02-28 14:23               ` Barros Pena, Belen
2014-02-28 10:42 ` Barros Pena, Belen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5E53D14CE4667A45B9A06760DE5D13D055DD79DE@ALA-MBB.corp.ad.wrs.com \
    --to=david.reyna@windriver.com \
    --cc=alexandru.damian@intel.com \
    --cc=belen.barros.pena@intel.com \
    --cc=paul.eggleton@linux.intel.com \
    --cc=toaster@yoctoproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.