Copy problems - followup

[Bauer]

In an attempt to solve my latest fabrik issue (in bold below), I came across this today in the Professional Support area - and it has me asking if this issue is being addressed?
Copy record (via form copy) will "unlink" all joined data from original record

IMO, this is the best example of the horror that comes out of using ?repeat? tables to store data rather than Json encoding the data and storing it all in the same table.

As far as I can tell, copying (or deleting) any rows from a table that has any ?repeat? tables associated with it is never being done properly.

When a row is deleted ? are any/all elements in a row that contains an element that uses a repeat table being checked for? ? and are the rows in the repeat table(s) being deleted also?

When a row is cloned/copied ? are any elements in that row that uses a repeat table being checked for? and are the rows in the repeat table(s) being created/copied also?

I'm pretty sure the answer to all those questions is NO. So of course ? copying a row from a table that contains an element that uses a repeat table isn?t really copying all of that data.

The last I tested it (just last week) ? what is happening is, when you copy a repeat group in a form via the form copy plugin, the new row id of the newly created row is used to replace the ?parent_id? in any/all of the repeat tables. This is not what you want to do - and explains how you ?lose? the joined data.

What really needs to be done in such a case (for copying rows) is to initiate a function that will?

1. Store the id of the original row being copied to a variable (e.g. $original_id).
2. Create the new parent table row/record and import the copied row data into it.
3. Save and store that new row id to a variable (e.g. as $new_id ? which will become the new ?parent_id? in the newly created repeat table rows done in #6 below).
4. Using a query on the INFORMATION _ SCHEMA . TABLES, get a list of all the ?repeat? tables used by the table from which a row is being copied.
5. Loop through that list ? and, using the $original_id as the WHERE condition ( e.g. ?WHERE parent_id=?.$original_id ), query each of the ?repeat? tables to produce an array of all the rows from the repeat table that contain the data for the copied row (that has that parent_id).
6. Then INSERT those query results into that repeat table ? using the $new_id for the ?parent_id?.
7. Repeat #5 until all the ?repeat? tables have been updated with the copied data for the element that repeat table represents.

A similar function/method would also need to be created to handle the deletion of rows in any table (joined or not) that contain any multiselect elements that use a ?repeat? table. This one is a lot easier?

1. Store the id of the original row being deleted to a variable (e.g. $delete_id).
2. Delete that row from the table.
3. Using a query on the INFORMATION _ SCHEMA . TABLES, get a list of all the ?repeat? tables used by the table from which a row is being copied.
4. Loop through that list of repeat tables ? and, using the $delete_id as the WHERE condition ( e.g. ?WHERE parent_id=?.$delete_id ), delete all rows from each of those ?repeat? tables that contain that ?parent_id? value.
5. But to really complicate matters - If the list has linked/joined tables and/or repeat groups ? you would have repeat this process/function on the joined table row(s) and/or repeat groups.

Otherwise, in a short time you may have ?repeat tables? that are loaded with orphaned rows from rows in the parent_table (parent_ids) that have been deleted.

And this, IMO, is the can of worms that these ?repeat tables? have created. ( I have one table that uses 18 of these ?repeat? tables ? and to make matters worse I?m sure, that table is used as part of a repeat group.)

I have already written a function to ?clean up? and handle orphaned repeat tables and/or repeat table rows that reference a non-existent ?parent_id? ? but that is not something that should really be needed, if delete and copy/clone functions were being done right to begin with. I have also already created the functions to handle copying or deleting a row with 'repeat' tables associated with it (per my outlines above). But it only handles this one form in this one instance.

I?d offer up my code but it is being done very old school ? i.e. using php string functions to identify the repeat tables and field names that any repeat table is using. Maybe the better way would be to use the fabrik ?element? table to identify those repeat tables. Though, I dunno ? since both would accomplish the same thing.

My awakening to all of this came about because I now have an instance where I want to copy ALL of the values from a row in a repeat group into a new ?Form Templates? (?form_templates?) table ? to allow the user to use those settings, as saved, as a ?template? for a later-added repeat group. Not only would that require this new ?form_templates? table to hold cloned values of the elements in the form from which a template is being created ? but it would require creating all of the associated ?repeat? tables ? e.g. a new ?form_templates_repeat___facility_types? table to hold the values from the database join element ?facility_types?. But, besides physically creating the new empty table structures for the tables (copied/renamed via mySql), then adding the new ?form_templates? table to fabrik - wouldn?t I have to add the ?repeat? tables to the #__fabrik_elements or #__fabrik-joins tables also? Or would they automatically get created when the new ?form_templates? parent table is added as a new fabrik table?

 

[Bauer]

To followup on this - Here's a function I wrote that checks for and deletes rows from repeat tables for any rows in a table that are deleted.

Calling this function from components/com_fabrik/models/list.php seems to take care of the issue of 'orphaned' repeat table rows/records.
I inserted it there twice -
1. in public function deleteRows and 2. in protected function deleteJoinedRows
(NOTE: There are no spaces anywhere in INFORMATION _ SCHEMA . TABLES - but for some strange reason I could not post until I added the spaces)

function deleteRepeats($parent_table, $parent_id){
    // Get Database Name
    $config = JFactory::getConfig();
    $dbname = $config->get('db');
 
    $db = JFactory::getDbo();
    $query = $db->getQuery(true);
 
    // Get list of repeat tables used by this table.
    try {
        $db->transactionStart();
        $query = $db->getQuery(true);   
        $query->select("TABLE_NAME")->from("INFORMATION _ SCHEMA . TABLES")
            ->where("TABLE_SCHEMA='".$dbname."' AND TABLE_NAME LIKE '%".$parent_table."_repeat_%'");
        $db->setQuery($query);
        $result = $db->loadColumn();
        $db->transactionCommit();
    }
    catch (Exception $e) {
        // catch any database errors.
        $db->transactionRollback();
        JErrorPage::render($e);
    }
    foreach($result as $table){
        $db = JFactory::getDbo();
        $db->setQuery("DELETE FROM ".$table." WHERE parent_id=".$parent_id);
        $db->execute();
    }
}

Here's the change to public function deleteRows() - near the bottom of the function where $db->excute is called

if (!$db->execute())
        {
            throw new Exception($db->getErrorMsg());
      // ADDED ELSE       
        }else{
            deleteRepeats($table->db_table_name, $key);
        }

and in protected function deleteJoinedRows() - just after the 'for' loop

 /// Check for repeat_tables
            $checkIds = explode(',',$val);
            foreach($checkIds as $idnum) {
                deleteRepeats($join->table_join,$idnum);
            }

I'm just realizing this would also have to be taken even further - i.e this deleteRepeats function would also need t0 be called from the functions where tables are dropped. And probably a few other places I may have missed or am unaware of - anywhere rows are removed or tables dropped..

 

[troester]

Can you open a new thread with recent issues on the latest version?
Delete is working, did you set "Delete joined data" in list settings (Data/Joins)?

 

[Bauer]

Can you open a new thread with recent issues on the latest version?
Delete is working, did you set "Delete joined data" in list settings (Data/Joins)?

Thanks for the reply, troester.
Just to let you know - I'm not ignoring you (or Hugh's) request. I've just had some health issues lately that keep from from getting into this. Once I'm feeling better I will re-test all of this and start a new thread regarding my findings.

As for the "Delete joined data" option. That may be working for joined tables that have been set up in the 'Data' settings of a Fabrik List.
But my question (the problem) is...
If a databasejoin element (or any row containing any) is deleted from the table, do the 'joined data' records that get created in the x_repeat_x table get deleted too? (Those have nothing to do with joined tables in a Fabrik List's Data settings)

And are those records that hold the dbjoin data in the x_repeat_x table getting created properly when a row is copied?
I'm pretty sure that is an issue that has never been fixed. I'm also pretty sure this same situation affects cascading dropdown elements too - which is why I couldn't figure out a way to handle/fix both situations the last time I tried ti fix it. (And, as far as I know, there may be (an)other Fabrik element(s) that create and use those x_repeat_x tables also.)

 

[cheesegrits]

When you say ...

If a databasejoin element (or any row containing any) is deleted from the table

... I'm not quite sure what you mean. I understand "any row containing any", but not sure what you mean without the parentheses, "if a databasejoin element is deleted from the table".

But to answer the "any row containing any" ... and assuming you mean "multi select" joins (like checkbox joins) that create a <main_table_name>_repeat_<element_name> many-to-many map table ... then yes, those do get deleted.

It happens in ./plugins/fabrik_element/databasejoin/databasejoin.php, in onDeleteRows(), around line 3955. I just tested, and when deleting a row in my fab_checkbox_join test table, which has a checkbox join to my 'states' table, and a checkbox CDD to my 'cities' table, it correctly did ...

DELETE
FROM `fab_checkbox_join_repeat_states`
WHERE id IN ('3','4')

... and ...

DELETE
FROM `fab_checkbox_join_repeat_cities`
WHERE id IN ('1','4','5','6')

... which were the two states (in the join checkbox) and four cities (in the CDD checkbox) I had select in the form for that row.

-- hugh

 

[cheesegrits]

Note that I did this by deleting from the list. I haven't tested using a 'delete' button in the form, although I'm pretty sure that fires the onDeleteRows plugin hook as well.

-- hugh

 

[Bauer]

Note that I did this by deleting from the list. I haven't tested using a 'delete' button in the form, although I'm pretty sure that fires the onDeleteRows plugin hook as well.

-- hugh

Thanks for taking some time to look into this, Hugh. I know you're a busy guy and I really appreciate that.

On my PC I have a history of unzipped folders containing Akeeba backups of the website for this project dating back to Sept. 7, 2015.
I just did a search for 'onDeleteRows' and there was no function by that name in ./plugins/fabrik_element/databasejoin/databasejoin.php at that time.

Then I did the same in the folders containing my most recent backup and saw that, while onDeleteRows is in databasejoin.php, it is not in cascadingdropdown.php (the other element that I remember having this same issue with repeat table records not getting deleted).

To test, I then deleted a row (as a user on the front end) from a list containing a cascadingdropdown element - and, although I'm not sure how or where it gets done, this is now also working when you delete rows containing cascading dropdown elements too!

So while I'm not aware when all that was fixed since when I first posted this thread - it's good to know that this has been fixed for both databasejoin and cascadingdropdown elements. (Hurray!!! )

OK - So then I moved on to the real issue being addressed in the title of this thread - which is the problem/bug in the List Copy plugin - where these "repeat" records used in either databasejoin or cascadingdropdown elements (and there may be other element types?) are not being copied.

I just tested again - THAT problem still exists.
So it sounds like you need a onCopyRows() function for these element types too.
(And yes, it needs to be checked that this would works for both the list and form 'Copy' plugins)

The code that I had already written a few years ago is a band-aid that fixes this issue after the fact.
But the correct way is to just do it right when the row is being copied.

In that 'onCopyRows()' function, you would just need to know the 'copyFromID' as well as the new PK ID of the copied row in the parent table. Then get an array of the repeat table rows with the 'copyFromID'' and change the 'parent_id' values in that array to the new PK of the copied row in the parent table - before inserting that array into the *repeat* table.

And finally, one issue I foresee with the copy plugin is a problem that might arise if the row is a read-only detail view. Will the data still be copied correctly? I suppose that would be what you call a 'corner case' issue - but I'm just wondering.

It doesn't seem too difficult, really - and I still can't believe that I'm the only one complaining about this long-standing bug.

As I have explained many times, if I could afford a higher support level, I would have done so long ago. I try to make up for the lack of funds with what I'd like to think are helpful suggestions or fixes for problems that I have encountered in my 4 years of using Fabrik - and, health permitting, by participation or assisting other Fabrik users in the Forum - or updating the Wiki, or even posting my code fixes at Github.

Thanks again for the greatest extension since the advent of Joomla itself - and your dedication to the Fabrik product!

 

[troester]

The copy button in form is working (tested with a dbjoin checkbox).

The copy list plugin is creating the correct number of records in xxx_repeat_element-name with the correct (new) parent_ids - but with wrong dbjoin values (so in list view it's showing nothing, in details view "unknown label").

 

[cheesegrits]

Yup, I can confirm that. It's using the id of the mapping table row being copied instead of the FK. So (in my case) copying a row with a 'states' checkbox join, if I copy ...

id, parent_id, state, params
6, 10, 43,

... and the parent_id becomes 11, the newly minted map table row is ...

7, 11, 6,

... instead of ...

7, 11, 43,

I'll see if I can track that down. That code is a little opaque, though.

-- hugh

 

[cheesegrits]

That was fun.

Should be fixed as of this commit:

https://github.com/Fabrik/fabrik/commit/73883e25be204aff99828a357c838240c679593c

... at least for use of the list copy row plugin. Haven't tried with the form copy button, but that should be OK. The problem was another manifestation of the difference between data formats between rendering (as it comes from the form model getData, which is what the list copy plugin uses to get the data for the rows being copied) and submission (as the data comes in from the form submission). When you feed getData() results into the form's processToDb(), some stuff has to be fettled into the right format. In this case, the join's _raw array contained the id's of the map table, rather than the selected FK values.

So I hope the form copy button should be OK, as I think that uses the submitted data, doesn't re-read it with getData.

-- hugh

 

[troester]

Ok for my record with dbjoin checkbox now.

 

[cheesegrits]

Yay!

I'm not entirely happy with the fix, it's a but of a hack, but unfortunately, the onCopyRow() element model method doesn't get fired for multiselect joins. So I had to hack it into onFinalStoreRow().

-- hugh

 

[Bauer]

Yay!

I'm not entirely happy with the fix, it's a but of a hack, but unfortunately, the onCopyRow() element model method doesn't get fired for multiselect joins. So I had to hack it into onFinalStoreRow().

-- hugh

Looks like as good a place to put some code for a special situation like this. Why do you call it a "hack".
The fact that you even knew the problem brings nothing but a "You the man", Hugh.
Thanks. I'll try to test myself tomorrow.

 

[Bauer]

Ok for my record with dbjoin checkbox now.

Excuse my absence today - and thanks for jumping in and testing, troester.
It's always good to read that it's not 'just me'.
I'm just updating from github now.

 

[Bauer]

The Form Copy button works.
The List Copy button/plugin is still not working correctly for elements that use *repeat* tables.

In that onFinalStoreRow() function that Hugh edited, the values for the dbj elements of the copied row (both label and raw) are always empty - so none of that new code is executed.

The actual new row gets created in the List just fine, but if I go into phpMyAdmin and look at the repeat tables right after the copy is made, the newly copied rows that should be there are not there. But what's really strange is if I immediately edit that newly copied row in form view, the selected values of the dbj element(s) are shown correctly for those elements.

HUH? Now where the h is that form of the newly created row getting the correctly selected values for the dbj element(s) if they were not even added into the *repeat* table???

Then of course, if I save that form view of the newly copied row (without editing a thing), the selected values then get created in the *repeat* table as you would have expected them to have been created before opening the row for editing.

So this one has my head spinning - as has been the case for over 3 years of trying to get this list copy plugin working with dbj elements.

I already spent over 2 hours trying to figure this out and got nowhere.
I have some domestic chores that need taken care of today per the "Make hay while the sun shines" rule. So I'll see if I can't come up with anything more helpful as soon as I can get back to this.

 

[troester]

I really can't replicate.

"repeat" elements (tested with dbjoin checkbox) (your-table_repeat_your-element-name)
"repeat" group created via list join (joined table)
"repeat" group created as "group repeatable" (your-table_groupId_repeat)
are all copied correctly now.

Are you running the complete latest GitHub version (without custom hacks)?

 

[Bauer]

I really can't replicate.

"repeat" elements (tested with dbjoin checkbox) (your-table_repeat_your-element-name)
"repeat" group created via list join (joined table)
"repeat" group created as "group repeatable" (your-table_groupId_repeat)
are all copied correctly now.

Are you running the complete latest GitHub version (without custom hacks)?

I have a few 'custom hacks' but I haven't made them since I extracted this latest github zip yesterday.

I'm not going to worry about it. It's too much for me to even try to figure out.
But the weirdness that is happening is just as I explained (and I'm willing to include screen snips of the list and forms if you don't believe me).

Like many of the lists in this project, this list is not your everyday Fabrik list. There are 5 databasejoin and 2 csacadingdropdown elements. Three of the elements make calls to different functions in user_ajax.php, if changed. And if any of the elements get changed, they affect a filtered databasejoin list of 'Included Facilities' and user-ajax must get called - then some javascript automatically sets/selects the proper 'Included Facilities' select options in one of the databasejoin multiselect elements (which is read-only to the user).

None of that is broken so it doesn't need fixing - but my guess is this list copy plugin quirk is most likely happening because of something unique to this list configuration.

So rather than beating myself up trying to get the List Copy plugin to work here, I will just add a Copy button to the form. That works! ...but there is still a problem.

These records are user-configured 'Custom Report' configurations that the user can edit only until they 'apply' the configuration. The intent here (using the list Copy button) was to allow the user to be able to use an existing record (that they had created) as a template for a new report configuration - then just tweak that rather than have to configure each Report from scratch.

But now, if I have to use the Form Copy instead, the rows that have already been 'applied' are no longer able to be edited - so they are no longer able to be copied and used as a template for a new Report configuration. ...Because the Copy button does not show/work in details view. I guess 'we' can live with that.

...OR, I'm thinking, I will just have to use the list php plugin and write the code for copying myself. I already will have to write some code to 'unflag' a few of the elements that act as yes/no flags - like Applied? (Yes/No) - and also need to null the 'date_applied' value. So I may as well just do it all with php.

Anyhow, thanks for your time and help.

 

[cheesegrits]

I've run the list copy plugin through all my torture test lists, with repeat groups, repeat joins, repeat CDDs, can't get it to fail.

So I'm about 98.486% sure it's something to do with your custom code.

The way the copy works is that it calls the form model's getData() on each of the row id's being copied, to get the data to then submit into a new row, through the form model's process(). If the repeat elements are coming up empty in onFinalStoreRow(), that means their data isn't getting selected in the getData(). Which probably means there's either a filter or ACL issue.

Or, it occurs to me, you mentioned that ...

then some javascript automatically sets/selects the proper 'Included Facilities' select options in one of the databasejoin multiselect elements (which is read-only to the user).

It's entirely possible that might be the problem. As the copying goes through the standard form model processing, it may well not copy data the user doesn't have write access to. Which is one of those "there is no right way to do that" things - your application may assume that we should copy data the user doesn't have write access on, someone else's app may assume the opposite.

So just for a test, if the elements which aren't copying properly have ACL's on them, try setting it to Public, and see if the problem goes away. Then at least we'd know.

-- hugh

 

[cheesegrits]

Hmm, nope, a quick test seems to show that we happily copy read-only elements (including repeat elements).

-- hugh

 

[Bauer]

I don't doubt for one minute that this is "just me" (just this list/form). Hearing testimonials from both you and troester is all I need to be convinced of that.
At least my initial observation wasn't all "just me" too - and something got corrected out of all this. Look at the bright side.

I will use far less of my time by just hard-coding what I need - triggered via a php list plugin button - and also includes the code to reset a few fields that need to be reset to their default values anyhow if an existing row is cloned. (I suppose that would make a nice option in the List Copy plugin configuration form - i.e. a multi-select of all elements in the table where those selected would get reset to their default value after copy/clone. That's probably easier to do than me finding what the hell I did to cause this weird problem. Maybe I can add that feature in appreciation for your time here - if/when I'm capable.)

Thanks again to both you and troester.