Databasejoin plugin WHERE clause not working

Funny enough I was discussing Admin stopping you from doing stupid things (like set Include in List Query to No for main table primary key) yesterday - so I know that Hugh / Rob believe in this also - but their time is limited and so putting these constraints in is not making it to the top of their priority list.
 
Well they have created a monster here - so I understand the priorities. Just sayin'.

From looking at the latest commits, it looks like Rob is addressing some of the database join issues. That's my biggest concern. So I couldn't be happier once this issue is resolved.:D
 
Just sayin' don't fix things.

I know its a little time consuming to track down bugs and work out how to fix them, but if we as technically competent users don't help by doing this, the reality is that they may never get fixed. Rob and Hugh only have so many hours in a day to work on this stuff.
 
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.:(

I have submitted a PR for fixing this in calc today - essentially the calc worked in repeating groups in 3.0 and was broken in 3.1 - but now it is fixed.

I can see if we can retrofit this to the Where clause in databasejoin - can you confirm that this is where the problem lies for you?

S
 
I have submitted a PR for fixing this in calc today - essentially the calc worked in repeating groups in 3.0 and was broken in 3.1 - but now it is fixed.

I can see if we can retrofit this to the Where clause in databasejoin - can you confirm that this is where the problem lies for you?

S
Yes I know exactly where the placeholder gets turned into a comma delimited string. It's done in the parseMessageForPlaceHolder( ) function found at line of components/com_fabrik/helpers/parent.php which is being called in plugins/fabrik_element/datbasejoin.php at lines 262, 853, 886, and 1018. (It's the call made from line 853 that triggered the mysql error for me - but the others, that are getting the label or concat, would need to be changed too)

I just think that fixing this issue in every plugin, as a bug report for that plugin comes up, is not the quickest solution. I'm assuming that is what was done to fix the calc function - i.e. pull the correct value out of the comma delimited array that is returned by the parseMessageForPlaceHolder( ) function.

But I'm pretty sure that for all plugins having this issue - it could be fixed once and for all if the parseMessageForPlaceHolder( ) function itself was able to identify the placeholder as an element in a repeat group and return the correct value to begin with - based on the current repeat group index (which would get passed as another parameter to the parseMessageForPlaceHolder( ) function).

I haven't looked real deep into that parent.php helper file so am not sure how this would/could be handled. But if the repeat group index isn't known or available to that class, then why not just add $repeatIndex = null as another parameter to the function? Then if a $repeatIndex value is passed to it (as '2' for example) then the value returned, instead of the comma delimited string containing an array of that placeholder's value in the list, would be the index key value of that list array (the repeat group it was in) - i.e. instead of returning 'dogs, cats, mice, rats, hens' it would return 'mice' ($element_array[2]) as the placeholder replacement value.

And I've been meaning to say a big THANK YOU Sophist for all your help in getting this issue settled.
 
I discussed adding functionality to parseMessageForPlaceHolder (specifically in index option which could be used when called from within a repeat group), but Rob was worried that this was too big a change, probably wasn't needed except in exceptional cases and asked me to fix the calc which was broken (and which I needed fixed for myself). So that is what I did.

I talked about the default eval, but Rob's view was that default is only used when you open a new record and that a new repeat group item would not need to refer to other fields in the same record because they would all be new. And I guess I agree with that.

But there are other elements that might need the same fix - Cascading Dropdown is one that immediately comes to mind. I am not quite sure that I understand why database join needs it. The database join where clause is executed when you add the new repeat group item - all fields in the new repeat group item are default (and therefore known) so you should be able to code the Where clause without needing to refer to them.

If you can explain to me why it is needed, I will see if I can find the time to address it.

S
 
I already explained somewhere in this thread - or the other thread about dbjoin - why I was using a placeholder from an element in the same repeat group. I needed the dbjoin element to be populated with all the rows from the joined table EXCEPT rows that already contained a value already being used in one of the other repeat groups.

In other words, the value for that dbjoin element had to be unique in each of the repeat groups. Pretty basic stuff, or so I thought.

But, if it was an edit , when populating the dbjoin - you would want the where clause to also include the existing value in the list returned - else the dbjoin link would never work.

So since the "Only when new" option only works for a new record in the parent list, I had to make it so the "Apply Where when" option was set to 'Both'. And so, in that case, I had to including an additional OR to also include the current value of the dbjoin for that repeat group being edited in the list of rows returned by the plugin. Or so I thought. Where else would I get that value to compare except the placeholder for that element?

I posted this before too - the actual Where clause I was using in a dbjoin element named 'facility_type_id' - contained in the list 'fb_breakout_groups'. The table used for the dbjoin is named 'fb_facility_types' - and I was getting the 'id' as the Value and the 'facility_type' as the Label.

This was written assuming that the values for the record from fb_breakout_groups (the same group the dbjoin element was in) would be the values returned from for that current row (repeat group) - not the array of values from the parent list! And it works fine if those values are used in lieu of the placeholders.

WHERE
SELECT fb_member_details_repeat_facility_checkboxes.facility_checkboxes AS fid FROM fb_member_details_repeat_facility_checkboxes
LEFT JOIN fb_member_details ON fb_member_details_repeat_facility_checkboxes.parent_id = fb_member_details.id
WHERE fb_member_details.user_id={fb_breakout_groups___user_id})
AND (fb_facility_types.id={fb_breakout_groups___facility_type_id_raw} OR fb_facility_types.id
NOT IN (SELECT fb_breakout_groups.facility_type_id FROM fb_breakout_groups WHERE fb_breakout_groups.report_id={fb_breakout_groups___report_id}))

Now to play devil's advocate here. If the rule is that the values returned by the placeholder of a repeat group is a comma delimited string of the array values from the parent list (as it is), then I could use a WHERE clause that would be a bit simpler...

WHERE
SELECT fb_member_details_repeat_facility_checkboxes.facility_checkboxes AS fid FROM fb_member_details_repeat_facility_checkboxes
LEFT JOIN fb_member_details ON fb_member_details_repeat_facility_checkboxes.parent_id = fb_member_details.id
WHERE fb_member_details.user_id={$my>id})
AND fb_facility_types.id NOT IN ({fb_breakout_groups___facility_type_id})

BUT - How do I filter that so the final AND clause also includes an OR (to also include the current value of the record being edited - so the dbjoin will be able to pick-up that currently linked value and work as expected)?

Maybe the solution for this lies with naming placeholders? Could there be a placeholder used exclusively for the currently selected repeat group values? e.g. {fb_breakout_groups___fb_repeat___id}

Mine is a complex example - but I'm sure there are instances where a user would need to filter the values returned by a dbjoin list (using the Where feature of dbjoin) that required comparing a value in the existing table/record the dbjoin element is in.

I can't see how Rob doesn't see this as a problem.:confused:
 
Well - if you have a table with potential values and you want to omit values from similar fields elsewhere in the repeating group, then having a singleton placeholder just for this entry is not going to help you - as you will not be able to find out what values are in use elsewhere in the repeat group.

You need the placeholders for all the similar fields in the repeat group and some way of ignoring the one for this particular field. The solution above is OK providing that all the other repeat group items have been saved to the database, however that is not certain.

I think I understand what you are trying for, but no idea how you could enhance Fabrik to do it. I guess the way you want to do this is outside the boundaries of what Fabrik can do natively.

As an alternative approach, why not get all the values from the DB and then use a JS function to look at the values in the repeat groups displayed and remove them from the dropbox lists.

S
 
Somehow, as I was writing that last post, I knew you would reply by telling me a way around my original challenge. (I did it another way - a week ago.)

And yes - my Where clause did provide me a way to "find out what values are in use elsewhere in the repeat group". It just made another query on the same table and used a NOT IN(SELECT....) to find and eliminate what was already in use.

When it comes to programming, there's always more than one way to skin a cat. But that really isn't the issue here. I'll just shut up already and hope someone else as a Standard or a Professional member raises the real issue. Maybe then it will finally have the weight and get the attention it deserves.
 
Or you could try to fix it. If you are trying to do that, people will help you.
So you're telling everyone in this forum to just shut up and try to fix the bugs they find themselves? Or is it just me? I think I've already explained in this thread, which thoroughly explains the issue, the ideas I have for how to fix this. But it is beyond my capabilities to do so. Else I wouldn't keep harping about it.
 
No I am not telling everyone in this forum to just shut up and try to fix the bugs they find themselves. I am not even telling you to just shut up and try to fix the bugs you find by yourself.

What I am saying is that your mindset and expectations about open source software are off target. This is free software and you need to set your expectations about quality, timeliness of fixes, and roles and responsibilities accordingly. In particular you should not expect the level of bug fixes from the core developers that you would expect from commercial software. Open Source software generally survives because of community effort, the willingness of one community member to support or help fix problems experienced by another, and by a positive community spirit which recognises both the benefits and consequences of open source and which works together to make it a success.

I find the Fabrik code tortuous and difficult to understand - so I put in dump statements and work out what is happening and then try to fix it. And if I still can't understand how it works and how to fix it I ask for help.

I think you would find that if you stopped pointing the finger of blame and started asking people to help you fix the bugs you find, then you might get further. You already do pretty good in diagnosing where the problems lie, and I think you could fix some of them if you had some help when you got stuck.

And my offer to deal with the Git stuff for any fixes you create still holds because I know from another project that the GutHub learning curve can be high.

S
 
No one but you seems to be listening to this discussion - but if you go back to my response in reply #27. I colored this in maroon for a reason...

Maybe the solution for this lies with naming placeholders? Could there be a placeholder used exclusively for the currently selected repeat group values? e.g. {fb_breakout_groups___fb_repeat___id}

This would make it easier for the parseMessageForPlaceHolder( ) function to know whether to return the comma delimited array string - or the value for the current repeat group index. And there would be no need to pass another special parameter to the parseMessageForPlaceHolder( ) function. (And it would probably make changing the code in that function less difficult as it would basically just mean adding another case/if statement checking to see if "_repeat_" was in the placeholder name) How about running that idea by Rob?
 
Yes - I am listening and responding.I did see your suggestion - and I did respond in #28 (by discussing singleton placeholders like this and why your proposed enhancement might not work).

I have also discussed a more general solution with Rob but he has indicated that these sorts of changes to placeholders are not what he wants to see - and for me there is no point in coding something that he will not merge in.

Of course there is nothing to stop you coding this anyway and using it as a bespoke enhancement to your own copy of Fabrik - that is part of what Open Source is all about.

S
 
We are in need of some funding.
More details.

Thank you.

Members online

No members online now.
Back
Top