Databasejoin plugin WHERE clause not working

[Bauer]

In certain conditions (see below), if the WHERE clause is set to ?Only on New? in the databasejoin element, the query on the databasejoin table does not include the WHERE clause.

Looking into this, I now know why ? I just have no idea how to fix it.

This is occurring when I click the Add icon in a joined form. The new record being added is to the joined table ? not the ?master? table used in the form. This is why it doesn't work.

Here's what I found...
In the protected function mustApplyWhere() in datbasejoin.php - The line...

$isnew = $app->input->get('rowid', 0) === 0;

will always return $isnew as false because $app->input->get('rowid') will return the rowid of the controlling (parent) form - so it is never zero.

Does anyone have any idea what value needs to be checked in that mustApplyWhere() function in order to get this to work properly when the ?add? is on a joined table in a form? It looks like $app->input->get('rowid', 0) just isn?t cutting it, in this case.

 

[Sophist]

I am unclear whether you are doing this the fabrik way.

As I understand it you have a form which displays a "master" table and an "Add" button to add to the "joined" table.

I think that Fabrik would normally treat the "joined" table as a repeating group. I presume you already have a group with elements matching the "joined" table, so if you set this to repeating you should get a "+" button to add repeating elements. Is this what you are wanting?

P

 

[Bauer]

No I have no additional "Add" button. This is all fabrik. (I have a problem getting the fabrik lingo and terminology correct sometimes)

The add button I am talking about is the one that shows in each repeating group (Shown as Plus and Minus really - to Add or Delete) This is nothing special I am doing.

It sounds like the [rowid] being returned from input->get in the mustApplyWhere() function ( $isnew = $app->input->get('rowid', 0) === 0; ) is the rowid from the group that contains the clicked add icon. So it will never be zero and recognized as "New" record.

All you need to do to test is add a database join element to a repeat group. Include a where clause, and set the "Apply where when" to "New". Now run the form containing the repeat group and add a new group. Did the where clause work for the database join element in the newly added repeat group? (It might work ok for the first repeat group, I'm not sure - so be sure you are at least adding a 2nd repeat group)

 

[Sophist]

So, yes it sounds like you are using a repeating group. Yesterday I finally tested my own repeating groups working fine when not multi-page (there were some js issues when using multi-page). So I am not sure why you are having difficulties.

$app->input is the URL string, so it is looking for a rowid=x part of the URL. If you are adding a repeating group item to an existing master record, then it will not be 0 and $isnew will be false, but if this is an entirely new record, then URL will not have a rowid and $isnew will probably be true.

Hope this helps.

P

 

[Bauer]

I guess it helps - in that you seem to understand the problem.

But the fact is - the "Apply where when" option of the databasejoin element does not work as one would expect it to work when you set it to "New" and are using the datbasejoin element in a repeating group. In this situation, it must still, in fact, insert a "New" record in the table, no? Is this another case where I'm being told 'it's a feature not a bug'?

I spent hours tracking down this issue this morning - and can tell you that $app->input is not simply returning the URL string. For me it returns an object 75,564 characters in length. (Try a var_dump if you don't believe me.) So if you need a fine example of overkill to retrieve a simple $_REQUEST variable - you have one right here.

And actually, I think I'm getting to the root of this long-standing bug.

Looking at the repeating form groups via Firebug, I notice that for all the values in the repeated groups (as I click the plus sign to add 3 new groups) - even though the fields for the newly added groups all appear to be blank - those repeat groups ALL show the 'value' attributes of each element (except the internal id) to be the value from the last repeat group (including the databasejoin element in question). That happens DESPITE the fact that "Copy values" is set to "No".

Again, if you don't believe me - try it. Open a form that has repeat groups and add a repeat group (set to NOT copy values) and have a look at the fields in that new form via Firebug. ALL the values are carried from the last group.

That explains why my "onLoad" javascript events for these elements isn't working as expected (and perhaps this is causing bugs in other plugins that use javascript?) - the values are not being seen as blank by javascript.

Yet if you submit the form, the validation runs as if those elements are indeed blank - and if there are no validations, the record is in fact saved with all the values as blank. Now go figure.

 

[Sophist]

Yes - I have seen issues of values being set even when copy values is set to No.

However, I don't think you are supposed to use the databasejoin element to join repeating groups. Certainly when I use it, it is much more for lookup of field values. Repeated groups are a group with its own table and a join in the List / Data / Joins tab.

 

[Sophist]

This sounds like confusion over the meaning of the term "Only on New" - you believe that when the databasejoin is in the repeating group, it should mean the repeating group record being new, whereas it actually seems to mean that the master record is new.

I am not sure that this is a bug - more nomenclature really.

The question is what Rob's original intent was (in this situation), and whether it actually makes more sense for it to refer to the containing group record rather than the master record?

 

[Bauer]

Well yes, if the parent table is a new record - the first repeat group will be new also - and it will work.
But you would think the 'only when new' should refer to the table containing the databasejoin, regardless if it was in a repeat group, no?
If this is not the intent, then you shouldn't be allowed to even use a databasejoin element in a repeat group - as it causes all sorts of issues the way it is now (see http://fabrikar.com/forums/index.php?threads/bug-oversight-or-by-design.34798/ ) - if you want to call that the 'fix' for the problem.

 

[Sophist]

I think that it could mean either. IMO I would say that your's is the more natural option - but that may or may not be Rob's intent.

 

[rob]

Its the parent's table's new record. Never really thought about this situation, if someone wants to provide a pull request I'd consider including it.....

 

[Sophist]

I suspect that coding a fix for this is beyond both my understanding of Fabrik internals and possibly Bauer's too. I am guessing that this probably checks for the id field being non-zero, so I guess we need to add more sophisticated code to decide which id field to check - which would be id of the table that the Group is associated with.

Rob - if you could code this, that would be great. If not, I can put it on my to-do list but realistically will be unlikely to get around to it.

S

 

[Bauer]

Actually, in tracking this down, I found exactly where the "WHERE" clause placeholders get changed to an array/string value - in the buildQueryWhere( ) function of databasejoin.php. I just have no idea how to find the repeat group index at that point so I could 'fix' the where clause. I don't think it will be that hard for someone like Rob, who fully understands all the fabrik class names and variables, to get that info - and if the databasejoin element is in a repeat group, pull the correct value from the that comma delimited array/string and replace it with the correct value based on the current repeat group index key.

That was my first thought anyhow - if there's a better solution, I'm sure Rob already knows what needs to be done.

So who's making the pull request? (hint, hint, Sophist)

 

[Sophist]

The Pull Request is created when you have developed and tested a code update that you want incorporated into the GitHub Repo.

So Rob is saying that we need to develop this fix and then submit it.

 

[Bauer]

The Pull Request is created when you have developed and tested a code update that you want incorporated into the GitHub Repo.

So Rob is saying that we need to develop this fix and then submit it.

OK - well as soon as someone can tell me how I can identify the current repeat group index as the databasejoin element is being loaded in the repeat group - I'll be happy to get to work at it. I suppose that means signing up at the Github - and the whole bit. Push me hard enough and I actually let go of some of my stubbornness.

 

[Sophist]

Actually, signing up to GitHub is not too bad. And the GitHub windows client is good as far as it goes. But my experience is that there is a bit of a learning curve on how best to use Git, and the GitHub windows client does seem to need regular Git command line help.

So, I am happy to save you the learning curve and do what I did before - if you send send me the updated files with your fixes I will create a fix in your name, upload it and create a PR.

 

[Bauer]

Its the parent's table's new record. Never really thought about this situation, if someone wants to provide a pull request I'd consider including it.....

This is much more than just a problem with the definition of "New".

If the database join element is being used in a repeat group, any {placeholders} in the where clause are being interpreted and returned as a comma-delimited string (from the array of all the values in each/every repeat group). That is being done regardless if the the 'When' option is set to New, Edit, or Both.

Fixing this is above my qualifications. So if you are not going to assign someone to work on fixing this, I would suggest you either...

1. Remove the ability to use placeholders in the Where clause
2. Remove the ability to use the databasejoin element in any repeat group

Otherwise you will have a lot of disgruntled customers when they discover the plugin is not all it is made out to be.

That said, I suspect databasejoin is not the only plugin that has this issue. I count 33 plugins that call this same parseMessageForPlaceHolder( ) function from components/com_fabrik/helpers/parent.php. And if they are all returning the placeholder as an array string when the plugin element is being used in a repeat group - then I'd say you have a real 'issue' on your hands that needs to be resolved - and my 1. 2. list above refers to all of them. Fixing this would probably solve a long list of issues/bugs being raised where the bug only exists 'sometimes' - (that sometimes is when the plugin is in a repeat group).

I know how to solve the problem "in simple English". But converting that simple English into php code is another story.
The parseMessageForPlaceHolder( ) function needs to determine if the placeholder is being used in a repeat group - and if so, it needs to identify the index of that repeat group and return the array index value for the placeholder - not a comma delimited string of the values in all the repeat groups like it is doing now.

My project is on hold until this issue can be resolved.

 

[Sophist]

The placeholders should IMO work as follows:
1. If they are referring from a parent to a repeating group they should be an array as at present (of all the repeating group values).
2. If they are referring to the same group or from a repeating group to a parent then they should have the single value of their own record or parent.

This is a significant change to code - and ROb would need to code it. I suggest that you try to contact him and get him to agree to fix it.

S

 

[Bauer]

Well the bug (I'll call it that anyhow) is very elusive. I don't know how many months I have been having issues with databasejoin and couldn't pinpoint the problem.

If for example the element being compared is a string to begin with (and so you put quotes around the placeholder in the WHERE clause of the databasejoin) - it won't even error out because you will be comparing 2 strings (even though the 'array' string being returned is not what you expected - ie. 'cats,dogs,mice' is still as string ). The only time it would be OK in that case is if there is only one repeat group (and 'cats' was returned). Any array of one element converted to a 'comma delimited string' has the correct value - and without any comma.

But if you are looking for a number, the string returned will be something like 2,2,3 and it will throw a mysql (or 500) error when it tries to compare the two values. (And yet again, if there was only one repeat group there would be no error - as 2 would be returned.)

I have no idea how I finally came upon and noticed that a comma delimited string was being compared. I?m pretty sure that most of the time I got a 500 error instead of the page that was showing the actual mysql query that caused the error ? and finally had a screen print that explained what was going on. And I suspect that was the case for most anyone else who encountered the problem.

I sure hope this issue gets tackled because IMO it explains a lot of the problems I've been scratching my head about when using fabrik's "flagship" plugin.

 

[Sophist]

If it is any help to you, I have confirmed that nested repeating groups are not supported. I know that is not the problem here, but just in case you are thinking of doing this....

 

[Bauer]

If it is any help to you, I have confirmed that nested repeating groups are not supported. I know that is not the problem here, but just in case you are thinking of doing this....

Yes, I found that out the hard way, early on. Now if only there could be a check routine added to the code when saving the List configuration form, to assure users don't do this "no-no". (But that's fodder for another thread altogether.) IMO, if you "can't" do something in fabrik, then the code should be written so that you are unable to do it, no? (As creating nested repeating groups is guaranteed to mess up the tables involved)