Hello,

I've recently started working on a Salsa integration project, and I've run into an issue. I don't know if I am making a mistake or if the documentation needs to be updated. I'm trying to execute a getObjects.sjs query with more than one condition and not having much luck.

According to the DEVELOPER.txt file: A condition should be in the format of "Condition=Value". If there are multiple conditions, this can be an array of conditions:
'#condition' => array('condition' => 'value', 'condition2' => 'value2');

I'm not having any trouble with single conditions, but multiple conditions as an array result in a SalsaScript error (TypeError: Cannot call method "trim" of undefined).

Example code:

$query = array(
  '#script' => 'getObjects.sjs',
  '#tables' => 'supporter',
  '#condition' => array('Last_Name' => 'Example','State' => 'MA'),
  '#fields' => 'First_Name,Last_Name,State',
  '#order' => 'Last_Name ASC',
  '#limit' => '20',
);
$salsa_result = salsa_api_query($query);
// Do something with $salsa_result here....

In a related question, is it possible to use anything other than "=" in a multiple condition? E.g.: in Salsa API terms: condition=Last_Modified>2012-01-07&condition=State=MA

Thank you,
Eric

CommentFileSizeAuthor
#5 getObjects-conditions-1472516-5.patch7.26 KBberdir

Comments

johnshortess’s picture

johnshortess
he/him
Primary language English
Location Fairfax, Virginia, USA
commented
Title: Array for #conditions » Add support for multiple conditions
Component: Documentation » Code
Assigned: Unassigned » johnshortess
Category: bug » feature

I just took over as maintainer of salsa_api, and I've mostly been focused so far on porting it to D7. The module doesn't currently support multiple conditions. I'm not sure if the previous maintainer planned to add that feature and never got around to it, or just never committed that branch of the code to d.o.

Adding support for multiple conditions is certainly doable (and is a great idea BTW), but is a bit trickier than it would otherwise be because the query is currently built inside salsa_api_query() as an associative array using the parameter as the key. But it could be built something like this instead:

$build_query_array = array(
array('parameter' => 'condition', 'value' => 'Last_Modified>2012-01-07'),
array('parameter' => 'condition', 'value' => 'State=MA'),
);

That approach would probably work pretty well with some much-needed refactoring that Berdir did a few weeks ago, that I *think* I pushed to 7.x-1.x-dev when I got maintainer privileges.

I'm in the middle of a big Salsa integration project myself, so I won't have time to get to this until DrupalCon at least. But feel free to reassign the issue to yourself and roll a patch if you'd like. Otherwise I'll get to it as soon as I can.

Thanks!

eric at nrd’s picture

eric at nrd commented

Well, I see the problem with salsa_api_query() now - it is dropping the condition key, and keeping only the value.

Starting on line 238 (at least for getObjects.sjs):

      foreach ($build_query_array as $item => $value) {
        if (is_array($value)) {
          foreach ($value as $i => $v) {
            $query .= $item .'='. $v .'&'; // This results in 'condition=value&'
          }
        }
        else {
          $query .= $item .'='. $value .'&';
        }
      }

This results in a query string that looks like:
object=supporter&condition=Example&condition=VA&limit=20&...

So far I'm having luck changing the foreach loop to look like:

          foreach ($value as $i => $v) {
            $query .= $item . '=' . $i .'='. $v .'&'; // This results in condition=key=value&
          }

I don't know if this will break any of the other arrays of query items though (#fields and #links in particular). I'm not familiar enough with Salsa yet to even know how #links is used. Anyhow, I will work on applying this pattern to other parts of salsa_api_query(), do some testing, and if all goes well, I'll submit a patch for it (D6 only).

The above method doesn't really help with conditions that are something other than "=". One approach I thought of was something like this:
'#condition' => array('Field_01' => '=value', 'Field_02' => '>value2');

We would also want to check each value to make sure it starts with either =, >, or < (and if not insert a =).

This is more orthogonal with the way #fields works, but it is certainly more complicated than:
'#condition' => array('Field_01=value', 'Field_02>value2');

I'm happy to jump in and start working on some of these issues, but I don't want to do anything that would be inconsistent with refactoring of the D7 version.

codewatson’s picture

codewatson commented

@ johnshortess

Thanks for taking over, i've not had any time to work on it nor has my organization had much need of this so it definitely fell by the wayside for me.

I can tell you that i probably did a lot of work on it that didnt get committed here, which probably also included working conditions, i'd have to look back to see.

How conditions are handled should probably be reworked, as if i remember i had a conversation with one of the api devs at salsa about what operators are allowed, and i know there were a few, possibly even outside of =, >, < (cant remember what they were now)

I see there is now a 7.x version, so i imagine any changes should be made there then back ported to the 6.x version. How the queries are built/run aren't affected by which version of drupal you are using (unless john made a lot of changes) so that should be pretty simple. Anyway, a stable 7.x version should probably get released first so that it can be back ported?

When i get a chance i'll post the most current version i have so you can see what progress i made on it.

johnshortess’s picture

johnshortess
he/him
Primary language English
Location Fairfax, Virginia, USA
commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Active » Postponed

Good to hear from you, @dwatson. I'd definitely be interested in seeing any uncommited code you have. And yes, the plan would definitely be to develop in 7.x and backport to 6.x if possible.

@Eric at NRD, according to my notes (which are a combination of the API documentation at salsalabs.com and notes from meetings/calls with the developers at Salsa Labs), the allowed conditions are =, !=, >, >=, <, <=, in/not in (with the argument a comma-delimited list of values), is empty/is not empty.

I'm postponing until I can see whether the improvements @Berdir is making in #1508474: New object oriented query API are going to change the way queries are built, or just how they're executed.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Postponed » Needs review
StatusFileSize
new getObjects-conditions-1472516-5.patch7.26 KB

Ok.

Here is a patch that adds a getObjects() method to the new SalsaAPI class, which you can use with salsa_api()->getObjects().

That has a sane API and supports multiple conditions, with multiple values and custom operators (including documentation for it).

Similar methods can be added for all other scripts/methods and then we can deprecate the salsa_api_query() function.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

This is a new API that does not break existing code and it seems to work fine here both with manual and automated testing. Commited.

johnshortess’s picture

johnshortess
he/him
Primary language English
Location Fairfax, Virginia, USA
commented

Thanks, Berdir. This looks great. We're just starting a new Salsa project here, so in the next week or two I should be able to take care of at least some of the scripts called in salsa_api_query(). Once that's done, should we rewrite salsa_api_query() to take advantage of the new methods, or just deprecate it and leave it to developers to call the new methods themselves?

I'll also update README.txt and DEVELOPERS.txt, and the project page.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I think we should just deprecate the old functions. The main advantage of the methods is that we are able to have different arguments and return values depending on the method.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Berdir committed 37bd3e7 on 7.x-1.x, 8.x-1.x 
    Issue #1472516 by Berdir: Added SalsaAPIInterface::getObjects() with...