Support for traditional Boolean operators

Bug #1152863 reported by Kathy Lussier
32
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Evergreen
Triaged
Wishlist
Unassigned

Bug Description

This enhancement will add a Boolean search tab to advanced search, which, when accessed, will allow users to enter a free-form Boolean search query using traditional Boolean operators (and, or, not). Evergreen locale settings are also leveraged to provide support for operators in other languages.

This functionality was important to many of our libraries because, even
though the advanced search interface provides graphical support for
Boolean searches, there is no way to control the nesting of searches,
making it difficult to perform more complex searches.

For example, if a user were to enter the search terms in Advanced Search
as drugs AND teenagers OR adolescents, the resulting query would be
((drugs && keyword:teenagers) || keyword:adolescents), when the intended
query is drugs && (teenagers || adolescents). Performing a search with
more complex nesting, something like "((mouse or rat) and trap) or
mousetrap", requires free-form entry to get the nesting right. However,
users are expected to know that && means "and" and that || means "or."
Our academics, in particular, were concerned, because they often start
their instruction in Boolean searching with the library catalog before
moving on to full-text databases.

A setting is available in config.tt2 to enable/disable this feature and to identify limiters that appear on the Boolean tab.

Still to come is support for wrapping an operator in quotation marks to treat it as a search term to allow for searches like:

"pride and prejudice" and (vampires or zombies)

where the first "and" is part of the quoted search term and the second "and" is converted to &&.

Catalyst IT did the work. The branch is available at:
http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/catalystit/boolean_search

I'll add a pullrequest tag once MassLNC has done some initial testing, but I wanted to post the code early on so that the community has an opportunity to review it too.

Tags: search
Revision history for this message
Dan Scott (denials) wrote :

After a quick read of the code, _prepare_biblio_search_boolean() concerns me; it's globbing and parsing files on disk for each and every search? Typically that would happen once, at startup time, and the results would be cached thereafter.

The location of the boolean localization files in the "/location/" directory seems a bit strange. Is that a placeholder for a better name?

Also, if I understand correctly, searching for "To be or not to be" is no longer going to result in highly relevant results for that phrase (as just a quick example that comes to mind). That concerns me, too, as a major change in behaviour.

Revision history for this message
Kathy Lussier (klussier) wrote :

Hey Dan,

Speaking to your last point, our expectation is that the user who clicks on the Boolean search tab would be a person more knowledgeable of search strategy than the typical user who is entering searches on the Basic or Advanced search pages. If they know enough about search to use those Boolean operators, I would expect they know enough to enclose a phrase like "to be or not to be" in quotation marks. When the final coding is complete, those quoted searches will continue to search as a phrase without any conversion of "or" to ||

I would never suggest replacing Evergreen's handling of those search terms in our current search interfaces. The ability to use "and," "or", "not" as search terms is something we value and would not want to lose.

Revision history for this message
Thomas Berezansky (tsbere) wrote :

I am concerned with the fact that the branch appears to be missing any and all new files? At least one template file seems to be outright missing right now.

I am also not sure that having different limiter options for advanced and boolean searches makes sense. Re-using the advanced limiter set seems like it would make the most sense there.

Revision history for this message
Dan Scott (denials) wrote :

Kathy: Right, thanks for the clarification. In the light of morning it's clear that the changes are meant to be limited to one interface. But in some ways I think that the goal is even weirder: the information literacy people think it's better to teach to one special interface that accepts a particular dialect of && / || / - instead of just teaching AND = && / OR = || / NOT = - which can be used almost everywhere else in Evergreen? Do they expect users to grok the Venn diagrams that are undoubtedly part of the Boolean logic lessons, but they don't think that those same users will be capable of understanding symbols as Boolean operators? I fear tears and recriminations as the users subsequently attempt to apply their lessons to the basic search box / advanced search box and encounter failure.

It seems to me that solving the root problems your academics identified (giving the advanced search box the ability to nest Boolean queries & providing contextual help that OR = || and NOT = - in this particular system) might be a better area to focus energies.

Thomas: Right; I noticed that last night but thought that perhaps the templates were still on their way. I echo your concern that introducing more potential interface inconsistency / configuration complexity via separate limiter options is not desirable.

Other notes:

* I don't think we really want the new $logger->info() calls in Search.pm.
* (nit) s/substatutes/substitutes/ in comments
* (nit) s/l18n/i18n/ (I think?) in comments
* (super minor nit) Two empty lines were removed in Search.pm -- arguably making the code harder to read because there is no separation between logical blocks of code--but in any case whitespace changes are typically kept in separate commits.

Changed in evergreen:
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Kathy Lussier (klussier) wrote :
Download full text (3.3 KiB)

Thanks Dan and Thomas!

I'll share the coding concerns with the developers, if they haven't seen them already.

Dan, you asked the question of whether we could just teach students AND = && / OR = || / NOT = -

The answer I've received repeatedly from our academics is 'no', and I can see their point. If it were just a matter of teaching NOT = -, there would be no problem as this has become a de facto standard on the Web. Using && and || is just not intuitive. In the case of the latter, you might need to first start the lesson by making sure everyone knows where the pipe key lives on the keyboard.

AND and OR (or whatever their equivalents are in other languages) are intuitive because they have real meaning in the language that the user speaks. You make the valid point that there might be frustration because the same search strategy doesn't work in the basic and advanced search interfaces. However, on the flip side, there might also be the frustration that the more intuitive operators can be used in all of the electronic resources provided by their library except the catalog.

You also suggested that we focus on giving the advanced search box the ability to nest Boolean queries. This focus was actually how we started thinking about the project. I looked through other search interfaces for examples on how we could support more complex nesting without adding further complication to the interface. While we had some ideas of ways to support more complexity in the nesting, each of these ideas seemed to further complicate the advanced search interface, making it more confusing not only for those who might want the complex nesting, but also for those who might just use the advanced search interface to add more limiters to the search.

Ultimately, my team here believed that manually entering complex Boolean queries with the more intuitive operators was more straightforward than improving the GUI to support the more complex nesting.

I'm not saying it's a perfect solution, and we certainly were open to working with the community to support the best implementation. However, when I posted a message about the project to the general list last month, the only feedback I received was a question regarding the release that would see this new feature and an e-mail about ongoing work for an Evergreen add-on search option that would also support traditional Boolean operators - http://markmail.org/message/n7j2s363jsj6asgp.

My hope was that if there were any concerns about this approach being "weird," that I would have heard it at that time BEFORE the coding had begun. If I had received this type of feedback at that time, I could have delayed the start of coding and then explored alternate approaches with the community. Given the feedback that I received, I assumed that people either a) liked the approach, b) didn't care or c) saw the bit about there being a setting to disable it and mentally made a note to disable it when the time came.

I hope you can understand that, at this point, we are limited in our ability to make major changes to those larger implementation details. If the additional config.tt2 options to identify limiters on the Boolean search tab are...

Read more...

Revision history for this message
Fredrick Parks (fparks) wrote :

I have just pushed the missing template, some how it didn't get added in the initial commit. I am also working on ignoring boolean operators that are inside a string literal [wrapped in quotes].

Revision history for this message
Dan Scott (denials) wrote :

I personally apologize for not reviewing the proposal earlier. I try to review everything I can as early as I can, but time is limited.

Could we perhaps drop support for lower-case versions of the operators, thus requiring upper-case, and then extend that support to all search interfaces? That would bring consistency to our advanced search syntax and avoid most of the copy-paste surprises that people would run into when they inadvertently include an operator, which are my primary concerns. And hopefully upper-case "OR" / "AND" / "NOT" would be considered teachable by the info lit people on your side.

It would also better match Google's operators in its advanced search syntax, and that tends to be a good thing:

* NOT: -prefix (which we already match)
* OR: OR
* AND: (ostensibly implicit, but "justin lise" provides much different results than "justin AND lise" -- and the results from the latter match '"justin" "lise"' , which is the new syntax from Google for "must include" that replaced the +prefix when they had to steal that for plus.google.com)

Revision history for this message
Kathy Lussier (klussier) wrote :

Hi Dan

We had considered the all caps option at one point too. If my memory serves me correctly, one of the reasons we decided against all caps was due to searches seen in our logs where users were entering their entire search in all caps. It certainly wasn't a majority of the searches, but we were concerned that using the all caps option would negatively impact those searches. I'm really not trying to be difficult, but I guess I would say we didn't want to be in a position where we were changing the search behavior for those traditional searches, particularly when there might be sites that don't want this functionality at all.

Kathy

Revision history for this message
Rogan Hamby (rogan-hamby) wrote :

I'm chiming in because I'm really interested in seeing this come into reality myself. I have a good number of power users who could really benefit from this.

I have to agree with Dan about the all caps however. Mirroring Google's behavior = good when it's both the baseline I have for teaching people and it's the same (or very similar) they have a better chance of retaining the knowledge of how to do it since they may use it elsewhere.

I understand the concern about all caps searches but I think the advantages of doing it as Dan proposes outweigh in value what you get from accommodating those users searching in all caps. I don't know what the numbers are but most patrons are learning that caps = yelling and avoid it out of a sense of aesthetics as much as anything.

Plus, I would rather have a system wide searching behavior than it's behavior restricted to a single place you have to navigate to, I think that lessens the value of a valuable change.

Revision history for this message
Fredrick Parks (fparks) wrote :

I have just bushed some updates for this feature.
- The keyword replacement now ignores and/or/not inside of quotes.
- Keywords are now being cached instead of read from disk on each search.
- $logger->info() calls have been removed except for one that feel is necessary incase of missing localization files.

As for some peoples concerns over the location/name of the directory for keeping localization files, I am not sure what would be the proper location/naming convention. I would appreciate input on this.

Thanks.

Revision history for this message
Pasi Kallinen (paxed) wrote :

Some things that pop in to my head right now:

It would be nice if we didn't have Yet Another File Containing Translations, especially a file that contains just 3 strings.

Are the strings picked up into any of the gettext catalogs?

It would also be nice (if those are folded into some other file), if there was come context to the translation keys. Instead of just "AND", should have something like "boolean_search.AND"

Revision history for this message
Kathy Lussier (klussier) wrote :

I took some time over the past week to discuss the all caps option with my team here.

There were mixed feelings about using all caps for Boolean operators, but, no matter what the opinion was on all caps, we would still like to see the separate Boolean tab implemented in tpac. I'm not sure if you were suggesting that the all caps be used in lieu of the Boolean search tab, but I just wanted to mention that we do find the separate tab to be easier to promote and teach to our users.

I also think it fits in well with those other tabs on the advanced search screen. These tabs are all for searching that will only be done by a small percentage of users and therefore doesn't distract the typical user from performing an easy keyword search. And sites do have the option in config.tt2 to disable this feature if they don't like the tab.

I also would like to share some of the concerns we had with the approach of upper-case operators being treated as Boolean operators in all search interfaces. It requires that users overcome the expectation that case doesn't matter in search. I think we've all been in a situation where we start typing and then realize we have accidentally hit our all caps key. If I'm on a user name or password field, I'll go back and re-type everything, but I may not bother to re-type my search terms in a search field because I just expect the system to handle the search the same way no matter what case was used.

I think the core of our concerns boils down to the fact that our consortia are supporting libraries that are very different from each other and are serving different user populations. While we do have some libraries that are looking forward to this feature, we have other smaller, public libraries that don't see it being used much by their populations and are concerned with anything that may disrupt searching done in the basic and advanced search interfaces, particularly when they are being used by people who may be less savvy with computers and more likely to enter a search in all caps.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

This branch needs a rebase on master. It apparently conflicts with the commit aaf2d7f5f4ecd7dd36d5aee5ced23de78712ff62

Thanks.

Revision history for this message
Mike Rylander (mrylander) wrote :

I'm finally getting to take a quick look at this, and I'm curious if any thought went into letting the existing code that already has these capabilities handle the string-mangling on a query-by-query basis (specifically, QueryParser).

When instantiating a QueryParser object, one can configure it to use any string for the supported boolean operators by passing the appropriate options. For instance:

  my $qp = OpenILS::Application::Storage::Driver::Pg::QueryParser->new(operator => { and => 'and ', or => 'or ', disallowed => 'not ' });

[Note the spaces inside the quoted strings above. Also, I tested this originally but not recently, but it is meant to work.]

This configuration could be loaded from anywhere, though the database would be preferable to the web server's filesystem and that would match existing functionality more closely.

The locale information is already passed all the way back to the appropriate Perl code in the storage app, and that code could look up (and cache) the alternate boolean operators for QP based on an API-level parameter, say something like a "friendly_bools => true" parameter that is a peer to the search org_unit parameter. That in turn would be triggered by an OPAC-level API call to the search service -- either an alternate API name for the high-level search method or a new parameter to the existing one.

Revision history for this message
Fredrick Parks (fparks) wrote :

Sorry about the delay. I have modified the code so now it is looking for the localized keywords in web/opac/locale/{location}/opac.dtd instead of the location directory I had made under templates/opac. I have also rebased with master. The new branch is located at user/catalystit/boolean_search.

Thanks for your patience.

Revision history for this message
Kathy Lussier (klussier) wrote :

The code is working for us as we expected, so I have signed off on the branch and am adding a pullrequest tag. http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/boolean_search.

With the sign-off, I added a commit to get rid of the separate Boolean search limiters and to use the existing advanced search limiters as was requested by Thomas and Dan. The added commit includes a release notes entry.

One thing I was unable to test was using translated strings as the Boolean search operators, so that's one piece that could use additional eyes

tags: added: pullrequest
Dan Wells (dbw2)
Changed in evergreen:
milestone: none → 2.5.0-m1
Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-m1 → 2.5.0-m2
Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-m2 → 2.5.0-alpha1
Remington Steed (rjs7)
Changed in evergreen:
milestone: 2.5.0-alpha1 → 2.5.0-alpha2
Revision history for this message
Pasi Kallinen (paxed) wrote :

The search filters on the Boolean Search page are the same as on Advanced Search, but have differing widths - on the Adv Search they're all the same width. I kind-of like the varying widths, except the language and video format ones are _too_ thin. Perhaps a min-width should be defined for all of them, and use the same code for Advanced Search?

I tested the translated strings, and they work as advertized.

Revision history for this message
Pasi Kallinen (paxed) wrote :

Typo in comment: "abreveation"

Revision history for this message
Fredrick Parks (fparks) wrote :

I have normalized the boolean search filters, so that they match the advanced search filters. The code is the same as advanced search so that if there are any changes to the structure it will be simple to port over to boolean search. Also if there are changes to the select box templates they will display the same on both search tabs.

I have pushed these changes to the working directory at: user/catalystit/boolean_search_normalized_search_filters
The branch has the commit from kmlussier and has been rebased to the current master.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-alpha2 → 2.5.0-beta1
Revision history for this message
Mike Rylander (mrylander) wrote :

With an extra release cycle to look at this again, I would like to re-raise my previous concerns about the approach taken here. Specifically, instead of using regular expressions to mangle a mechanically constructed query string at the surface (it's a rule: users will always find a way to confuse complex REs), leverage existing functionality meant for exactly this purpose. (See comment 14 above.)

Aside from my bias toward that particular implementation path, I'm very worried about the use of translation strings here. Their use here is akin to embedding html elements in translatable strings. You're mixing the concerns and purposes of code and translation markers in ways that are well outside the existing use pattern in Evergreen (and, in my understanding and opinion, against recommended i18n design patterns in general), and my fear is that someone down the road will accidentally break this feature because they didn't realize that those translation markers were special in some way, and that breakage will persist for quite a while. This is likely because working on the OPAC should not mean thinking about search syntax and parsing; you think about that when you're working on the search backend and query parser.

If, instead, the search code was intentionally taught to know about and deal with alternate boolean operator spellings, those working on the search code would assist in its maintenance, and those working on the OPAC would not have to avoid breaking the functionality.

In other words, to be blunt for the purpose of clarity, the brittleness of the design and the layer at which it is implemented mean that this will not likely be well-maintained in the future by those that work on the search /or/ user-interface portions of Evergreen.

Even if nothing is done to address the design and implementation -- I'm not the only committer, so another may pick this up to merge -- I would appreciate some discussion of these concerns. If anyone believes I am going overboard and seeing issues where there are none -- I'm only human -- I would be happy to hear why. I'm more than willing to be convinced that the approach here is the best we can or should do, but I currently feel pretty strongly that we can do better. I also believe it would not take any more code change (252 line inserted or deleted by the signed-off branch) to implement, and perhaps less.

Thanks, and please chalk up any negative tone to the medium. I'm only posting to discuss the code, not to attack the implementors.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

FWIW, I produced a branch merged with latest master.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/dyrcona/boolean_search_normalized_search_filters

Note that I "merged." I did not rebase or cherry-pick. I did the above because there were conflicts with master. I have been loading this on my development VM for Kathy Lussier.

Mike's comments give me cause for concern, and I am not prepared to commit this to master against his judgment.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-beta1 → 2.5.0-rc
Revision history for this message
Fredrick Parks (fparks) wrote :

The approach we took to create this was from the prospective of a light weight plug-in that has no impact to the underlying systems in Evergreen. Our goal was to not change how the other searches work, but instead try to implement the functionality in a way that would not require a change to query parser. The reason for this was to not change how the current “power users” used Evergreen.

In short, we didn’t want to change how something works for everyone for the request coming from a subset of the community (MassLNC). If it turns out however that the majority of the community would want to use language “and/or/not” vs “&&/||/-“ then the change to query parser would make the most sense.

I would suggest I18n be implemented in the front end of any project. The underlying business logic should never depend on what language is being use. This is copying how .dtd is currently being used in other areas of Evergreen.

Let me know if there are any questions or suggestions about the above.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.5.0-rc → 2.next
Revision history for this message
Dan Wells (dbw2) wrote :

I don't think anyone is suggesting deep changes to Evergreen internals. As Mike points out, the query parser code was already designed with configurable operators in mind. I agree that it makes the most sense to tap into this potential rather than try a wrapper-style approach.

It's entirely possible that these configuration options will need some troubleshooting and bugfixes to make work; they have been (as far as I know) sitting unused for quite some time. If one were to run into issues going this route, I think many here would be happy to help work out the kinks.

Revision history for this message
Scott Myers (smyers) wrote :

I can see a couple of problems with going with the query parser implementation.

1. The only search that needs a changed query parser is the boolean search which would mean needing to pass a flag all the way from the search page to the query parser which would be fragile for people unaware of it.

2. The logic on when to change the "and" to && is different for boolean search to handle the following correctly.
     "Lilo and Stitch"
   In the boolean search any qutoed search item gets searched on exactly as entered by the user. There is no functionality that matches that in query parser currently as I know it.

3. The current implementation works.

If we were to use the query parser to handle the boolean search these items would necessitate making a some changes to a part of the code that has been thoroughly tested.

With that in mind if moving the logic into query parser will get this into master so MassLNC can use it we would be happy to do so.

Revision history for this message
Kathy Lussier (klussier) wrote :

I just want to make a comment that the "Lilo and Stitch" example Scott cited is important to maintain in whatever solution we come up with. Once a user puts those quotation marks around search terms, the expectation is that the quoted portion will be searched as a phrase. This worked as expected in my testing of the original branch.

Revision history for this message
Mike Rylander (mrylander) wrote :
Download full text (5.0 KiB)

Scott and Fredrick, thanks for following up on this. I appreciate you taking the time to lay out your thoughts. I'll copy and paste from above for clarity.

fparks commented above:

> In short, we didn’t want to change how something works for everyone for the request coming from a subset of the community (MassLNC). If it turns out however that the majority of the community would want to use language “and/or/not” vs “&&/||/-“ then the change to query parser would make the most sense.

I understand that desire, and even empathize with it, but the rest of the community is then burdened with not breaking MassLNC's requested feature, especially because MassLNC always tries to work within the community -- or, MassLNC is burdened with unbreaking it when others adjust adjacent code. It is, perhaps, a difference of style, but I would start from the other end and say, "this looks generally useful, and there is code to support this already in an integrated way, so let's start from the position of a fully-baked feature."

But, excepting everything I just said, it seem clear that other users certainly would like something that looks like this feature, as evidenced by the activity on this bug -- there are comments receptive to the concept of the feature appearing with 10 days of the initial post.. IMO, we passed the tipping point pretty quickly.

> I would suggest I18n be implemented in the front end of any project. The underlying business logic should never depend on what language is being use. This is copying how .dtd is currently being used in other areas of Evergreen.

All other locations that use DTDs in Evergreen do so through an XML processor. I don't think there is any other place in the code where the contents of a DTD are being used to drive logic. You're generating regexp's based on DTD content, and while the case of English seems safe, others may very well not be. IMO, and accepting that perhaps we're misunderstanding each other, it isn't a correct statement to say that your use of DTDs is the same as other uses in Evergreen.

To sum up my response to fparks, I'm also in favor of the functional request, but my concerns remain about the implementation.

Later, smyers wrote:

> I can see a couple of problems with going with the query parser implementation.
>
> 1. The only search that needs a changed query parser is the boolean search which would mean needing to pass a flag all the way from the search page to the query parser which would be fragile for people unaware of it.

I see that as a straw-man, as it assumes that is either novel or the only available mechanism. First, there are many fields passed along with a search, and second, the search can be made self-configuring -- I can imagine a query modifier (say, #human_boolean) turning this functionality on for a specific scope within the search.

> 2. The logic on when to change the "and" to && is different for boolean search to handle the following correctly.
> "Lilo and Stitch"
> In the boolean search any qutoed search item gets searched on exactly as entered by the user. There is no functionality that matches that in query parser currently as I know it.
>

That's another s...

Read more...

Revision history for this message
Fredrick Parks (fparks) wrote :

I have just pushed up a new branch that uses existing functionality in the query parser to set key words as operators.
working/user/catalystit/boolean_search_query_parser (b22d7f54fc)

Currently I only have English keywords hard coded. I plan on adding support for foreign keywords next, and would appreciate some input on what the best way of implementing that would be.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.6.0-alpha1 → 2.6.0-beta1
Revision history for this message
Fredrick Parks (fparks) wrote :

I have just pushed up foreign keyword translation functionality to working/user/catalystit/boolean_search_query_parser.
I also rebased to master before pushing, so the history has changed.

Revision history for this message
Mike Rylander (mrylander) wrote :

Fredrick,

Is there a reason that you did not simply create a string translation table and leverage the existing i18n code? For instance:

create table config.strings (
  purpose text primary key, -- store a use case label here
  string text not null -- store the en-US version here
);

insert into config.strings (type,string) values ('bool-op-and',' and ');
insert into config.i18n_core (fq_field,identity_value,translation,string) values ('es.string',' and ','fr-CA',' et ');

insert into config.strings (type,string) values ('bool-op-or',' or ');
insert into config.i18n_core (fq_field,identity_value,translation,string) values ('es.string',' or ','fr-CA',' ou ');

insert into config.strings (type,string) values ('bool-op-not',' not ');
insert into config.i18n_core (fq_field,identity_value,translation,string) values ('es.string',' not ','fr-CA',' pas ');

With an appropriately shaped set of seed data. Then, in the IDL:

    <class id="es" controller="open-ils.cstore" oils_obj:fieldmapper="config::strings" reporter:label="General translations">
        <fields oils_persist:primary="purpose">
            <field reporter:label="Purpose" name="purpose" reporter:datatype="text"/>
            <field reporter:label="Key String" name="string" reporter:datatype="text" oils_persist:i18n="true"/>
        </fields>
        <links/>
        <permacrud xmlns="http://open-ils.org/spec/opensrf/IDL/permacrud/v1">
            <actions>
                <create permission="ADMIN_STRINGS" global_required="true"/>
                <retrieve/>
                <update permission="ADMIN_STRINGS" global_required="true"/>
                <delete permission="ADMIN_STRINGS" global_required="true"/>
            </actions>
        </permacrud>
    </class>

And then, in metabib.pm, something like:

if ($args{_boolean} eq 'true') {
  for my $op ( @{ new_editor->search_config_strings( {purpose => {like => 'bool-op-%'}} ) } ) {
    (my $key = $op->purpose) =~ s/^bool-op-//;
    $parser->operator( $key => $op->string );
  }
}

And in EGCatLoader just go back to setting the _boolean arg to "true".

Then you're not introducing more stored procedures, and a trivally simple UI can be built on top of config.strings -- see basically any admin interface for an example. This would also leverage existing code that is well tested, and we'd get a place to store more general translations.

Sorry if that wasn't clear the other day when you asked privately in IRC for guidance ...

Regardless of the implementation maintenance cost of your current branch, though, the patch is lacking seed data and IDL entries for the new tables, as well as a way to control the translations.

Revision history for this message
Fredrick Parks (fparks) wrote :

I have just pushed up a commit to my earlier branch with the changes Mike outlined in the last comment. I have also pushed a fix that addresses an issue with the operators not being reset in query parser after a boolean search.

However there is currently an issue with the localization. I think that the database call to retrieve the localized keywords is only receiving the code 'en-US.' I'm not sure if it's because the session_locale is unset when the query gets run, or what all the consequences would be if the value is set.

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.6.0-beta1 → 2.next
Revision history for this message
Dan Reuther (dreuther-deactivatedaccount-deactivatedaccount) wrote :

Hello. I am taking over work on this feature from Fred. The code is currently in a state that reflects Mike Rylanders suggestions on 2014-2-13 but we are experiencing an issue with the localization. Fred went into this on his last post briefly. Things work as expected when language is set to English. Upon switching to French Canadian the search fails. The operators are not translated. For example in English we search for 'book or moon' and get the expected result. In French searching for 'book ou moon' produces no matches but 'book or moon' provides the expected results.

Also logging the locale in metabib.pm shows that it is always en-US regardless of the setting on the web page. I am still trying to grasp this codebase and don't fully understand how the localization system works. I would appreciate any feedback or ideas anyone has on this.

Thanks

Revision history for this message
Dan Reuther (dreuther-deactivatedaccount-deactivatedaccount) wrote :

I have just pushed a commit to working/user/catalystit/boolean_search_query_parser that adds a UI element to the staff client for the config.strings table. This commit, along with the previous work that Fred did, should have everything implemented that Mike had out lined in his last post.

While there is still the outstanding bug that Fred and I have described and I am continuing to work on locating that I would appreciate, time allowing, if someone could look over what has been pushed and provide feedback on this.

Kathy Lussier (klussier)
tags: removed: pullrequest
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.