Field Qualification PATCH for DBIx::SearchBuilder and numerous other (hints) of patches to come

Hello Jesse, All!

While I was doing some development on RT I realized that I could not
pass SQL function calls to SearchBuilder’s OrderByCols and GroupByCols
functions that had Function(‘somparam’, fieldname) through to the DB.
In doing this I realized that these two *Cols functions utilized very
similar code that needed to be fixed up. The attached patch creates an
internal _qualifyField function that does this work.

I also found one other place where QualifiedFields was being blindly
set, so I changed that to use this routine as well. Note that I added
an (currently unneeded by my code) QUALIFIED flag that could be passed
through to this function. It would avoid any qualification and utilize
the FIELD param as passed by the caller.

Note that while doing this I realized that you also had a SUBCLAUSE
param used in Limit sub probably for dealing with similar cases in other
parts of the code. I don’t know which approach you like better, but the
attached patch has helped me do the queries I needed to do.

I do have some other mods coming that I’d like to submit for
consideration / inclusion. I am somewhat in a flurry of innovation, at
the same time as trying to upgrade RT here. I hope that these patches
will be well received, but at the same time I don’t want to overwhelm
Jesse and all with bulk patches. I think they will provide useful
functionality. To give you some idea of what I’ve been up to, here are
some of the highlights of what I am doing:

  • I have added a “Group By Columns” selection box to the search
    interface. It allows you to pick multiple columns that you wish to
    “Group” the results by. It ends up producing results such as:

Queue: corpsec-policy

Nobody - open

1487	Please push firewall policy 	open

corpsec-policy Nobody 0
user@no$uch.com 11 hours ago
11 hours ago 0

bdowling - new

1376	RE: Need to allow access  	new

corpsec-policy bdowling 0
joe@thi$.com 1 weeks ago
1 weeks ago 0

In the above case, I grouped by queue, owner and status. Currently the
first field is considered the “Master group by” and displayed with a
prominent heading. I have been considering adding a selector to how
many fields should be in that first heading. Note that you can pick
from many other fields as well. You can also pick date columns, such as
last update time (absolute) or created time and it’ll group them based
on the “Time Scale” that you select in a second dropdown (that I haven’t
added to the UI yet). E.g if you pick “Month” you will see the results
grouped by Month. It accepts Week, Day, Year as well. I thought this
functionality was nice as it adds a little more “reporting capability”
to the search interface and people don’t have to resort to external
tools for “what is each member of my staff working on, etc”.

  • I was confused by the “Tickets” heading and the “Query Builder” both
    bringing me to the same place. “Tickets” used to show you the search
    results, but now there is a separate “Search Results” Tab – after you
    are in “Tickets.” For now I removed the “Query Builder” link and
    changed “Tickets” heading to “Ticket Search.” I would like to find a
    way when there is a current search in the session to always display the
    “Search Results” entry, but I have not delved into this. IMHO the
    search Builder, while much more flexible, is a bit ominous. I am not
    sure if we should consider having “hidden” advanced options and provide
    a more basic search interface for the quick searches?

  • To support the above grouping, I made some changes to the
    AvailableColumns list, in 3.4.1 some of the columns appear with the same
    name, e.g. two “Created” fields. I adjusted the ones that are absolute
    time to be labeled “X Time” and left the relative time names alone.

  • I added very basic media=“print” Stylesheet support. What it
    basically does is remove any UI content, such as the left menu or the
    topbar quick ticket creation stuff. On the results page I also have a
    hidden (to the display) box that presents the QueryDetails, timestamp,
    Query etc, making the printouts more useful in that you know what it is
    showing and how to reproduce it later if need be. This was written
    quickly and is fairly rudimentary, but it works.

  • I also added some support functions to email address entry fields.
    E.g. if you start typing an email addresses into a box (currently min 3
    chars), it’ll go back out to the server and do a search for matching
    usernames and show you a server-generated autocompletion drop-down. It
    understands comma separated addresses, etc and will help you
    auto-complete each individual part. It is capable of searching the
    fullname as well as the email address. (Actually it could search on
    anything, helpdesks might like typing in phone extensions they see on
    caller ID for example.)

Yes, this is JavaScript based, and I know I am going to get an earful
about this. But I think it is absolutely acceptable for the app to
utilize JavaScript in ways that improve the user experience, but do not
hinder the application from being accessible to downlevel browsers.
This code leverages work of others on the 'net and it works very fast in
my testing. It will soon go into production in my environment.

I really feel that this streamlines using RT when you have to enter
addresses and helps to avoid some of the typos that I have seen happen
with these fields. Jesse – I know your reluctance on using JavaScript.
I hope that you do consider this patch as adding functionality to RT
that will help it gain further acceptance by enterprises and get them to
replace aging fat client apps. This patch was done in a fairly
CleanlyCustomizeRT™ “Add on Module” sort of way, but there may be a
couple of added callbacks added to other components to make this work
fully as a Module. I added a small little screenshot of this to give
people a idea how it looks.

Again, as I mentioned I am in somewhat of a fast paced development cycle
trying to ready this version for production. I hope to be able to
provide some of these patches soon. Some of them are not as integrated
with the others, and some are tightly integrated so I am not sure how
best to provide such bulk patches. Obviously many of these changes
required taking copies of the original components and editing them in
local/ mode. I welcome your thoughts, comments on these .

As always, Jesse, thanks for a great product.

Cheers,
Brian Dowling

SearchBuilder.pm.patch (3.89 KB)

Hello Jesse, All!

While I was doing some development on RT I realized that I could not
pass SQL function calls to SearchBuilder’s OrderByCols and GroupByCols
functions that had Function(‘somparam’, fieldname) through to the DB.
In doing this I realized that these two *Cols functions utilized very
similar code that needed to be fixed up. The attached patch creates an
internal _qualifyField function that does this work.

Thanks! I’m going to tweak a bit before I apply. (Mostly naming stuff).

I also found one other place where QualifiedFields was being blindly
set, so I changed that to use this routine as well. Note that I added
an (currently unneeded by my code) QUALIFIED flag that could be passed
through to this function. It would avoid any qualification and utilize
the FIELD param as passed by the caller.

nod I’m not quite sure. I sort of feel like we should add it the first
time we need it. That make any sense?

  • I have added a “Group By Columns” selection box to the search
    interface. It allows you to pick multiple columns that you wish to
    “Group” the results by. It ends up producing results such as:

Very nice! I’ve wanted this for a long, long time.

  • I was confused by the “Tickets” heading and the “Query Builder” both
    bringing me to the same place. “Tickets” used to show you the search
    results, but now there is a separate “Search Results” Tab – after you
    are in “Tickets.” For now I removed the “Query Builder” link and
    changed “Tickets” heading to “Ticket Search.” I would like to find a
    way when there is a current search in the session to always display the
    “Search Results” entry, but I have not delved into this. IMHO the
    search Builder, while much more flexible, is a bit ominous. I am not
    sure if we should consider having “hidden” advanced options and provide
    a more basic search interface for the quick searches?

nod Have a look at what we’re doing with the “Simple Search” in the
quebec branch. This is something else that would benefit a great deal
from AJAXy progressive searches :wink: I agree that having a more friendly
beginning/intermediate search is the right thing.

  • To support the above grouping, I made some changes to the
    AvailableColumns list, in 3.4.1 some of the columns appear with the same
    name, e.g. two “Created” fields. I adjusted the ones that are absolute
    time to be labeled “X Time” and left the relative time names alone.

That’s something that should already be fixed in 3.4.2 that we got out
today.

  • I added very basic media=“print” Stylesheet support. What it
    basically does is remove any UI content, such as the left menu or the
    topbar quick ticket creation stuff. On the results page I also have a
    hidden (to the display) box that presents the QueryDetails, timestamp,
    Query etc, making the printouts more useful in that you know what it is
    showing and how to reproduce it later if need be. This was written
    quickly and is fairly rudimentary, but it works.

Nice. I’d love to see it.

  • I also added some support functions to email address entry fields.
    E.g. if you start typing an email addresses into a box (currently min 3
    chars), it’ll go back out to the server and do a search for matching
    usernames and show you a server-generated autocompletion drop-down. It
    understands comma separated addresses, etc and will help you
    auto-complete each individual part. It is capable of searching the
    fullname as well as the email address. (Actually it could search on
    anything, helpdesks might like typing in phone extensions they see on
    caller ID for example.)

I want it. Lots.

Yes, this is JavaScript based, and I know I am going to get an earful
about this.

Not from me. My big thing about technologies like javascript is that
they not degrade the experience for folks who aren’t using them.
(You’ll see a bit of Javascript used to create combobox fields for RT
3.6)

This patch was done in a fairly
CleanlyCustomizeRT™ “Add on Module” sort of way, but there may be a
couple of added callbacks added to other components to make this work
fully as a Module.

I’d love to see how you did it. Have you seen the RT-KeyBindings
javascript addon I stuck up on CPAN? It’s something that should really
get added to RT’s core, but it will take a fair bit of work to walk
through the UI and add bindings everywhere :wink:

Best,

Jesse

Brian,

I tried your patch today. It seems like there’s soemthing not quite
right going on. A couple typos were easy to clean up, but it looks like
it sends RT into a tailspin when I run “make regression” on a clean
source tree. How much have you pushed at RT with your patch applied?

Best,
Jesse

Jesse,

Many apologies. That was my fault. I had it all working as I needed
it, and then I continued to make other tweaks and used qualifiedFields
function in other places I felt it needed it. It was “still working”
when I was done. Unfortunately I fell pray to the fact that I was
coding with DevelMode => 1 on and trusting that everything was magically
reloading just fine while I was making changes. Until I broke it with
syntax errors – RT kept working, so I thought my added changes were
fine, but I didn’t realize that this module was never being loaded until
I later restarted Apache. Again, I’m sorry for this. Attached is a new
patch that fixes these issues. As you said, there were a few minor
syntax errors, and I left a QUALIFIED flag in flipped logic used while I
was testing it.

(I also overlooked your response below. I only noticed it when I was
starting to resubmit this patch. :wink:

(Note this is still against 1.26; I haven’t upgraded to DBiX::SB 1.27
yet.)

Cheers,
Brian

Attached is the real diff, here is a shorter diff of the changes from
the last patch.

< return undef if (ref($rowhash) ne ‘ARRAY’);

return undef if (ref($rowhash) ne 'HASH');

338,339c338,339
< my $clause;;
< unless ( $rowhash->{‘QUALIFIED’} ) {

my $clause;
if ( $rowhash->{'QUALIFIED'} ) {

341,342c341
< } elseif ( $rowhash->{‘ALIAS’} and $rowhash->{‘FIELD’} ) {
<

} elsif ( $rowhash->{'ALIAS'} and $rowhash->{'FIELD'} ) {

344d342
<
370,371c368
< $clause = $rowhash->{‘ALIAS’} . “.”
< if ($rowhash->{‘ALIAS’} and $rowhash->{‘QUALIFIED’});

  $clause = $rowhash->{'ALIAS'} . "." if ($rowhash->{'ALIAS'});

1057d1053
< QUALIFIED => 1,
1132a1129

SearchBuilder.pm.patch.v2 (3.92 KB)

Jesse,

Many apologies. That was my fault. I had it all working as I needed
it, and then I continued to make other tweaks and used qualifiedFields
function in other places I felt it needed it. It was “still working”
when I was done. Unfortunately I fell pray to the fact that I was

Brian,

Does this work right for you when you run RT's "make

regression"? I’m seeing it generate ORDER BY / GROUP BY clauses
without the “main.” aliases. Which generates test failures.

Jesse