Add support for event instances #56

michael-bentz · 2021-06-28T19:31:36Z

ChemiKyle · 2021-06-29T14:13:03Z

Unfortunately, this results in APF not working across events or in repeat instances of the same event when using the XML in this comment: #24 (comment), which does use "Repeat Entire Event"

michael-bentz · 2021-06-29T14:15:17Z

Strange, that's the XML I used for testing.

ChemiKyle · 2021-06-29T14:29:39Z

In the existing record 1, I went to the Baseline Data form in Visit 1 (event3), without this PR active the fields should populate.

I also spawned a new instance for the Baseline Data form in Dose 1 (event2) and the fields did not autopopulate there either.

ChemiKyle · 2021-06-29T14:38:28Z

ExternalModule.php

+                    $prev_event_field_value = $data[$prev_event][$source_field];
+                    if (isset($prev_event_field_value) && !empty($prev_event_field_value)) {
+                        $default_value = $prev_event_field_value;
+                    } elseif ($data['repeat_instances'][$prev_event][$source_form]) {


This change is the source of the breakage for cross-event autopopulation, passing $source_form instead of "" results in null

I see, can you try the changes again with this setting enabled?

Same results, though that gets a little funky with imported data (timestamps for data entry are all the same when you do an upload), to really test it you've got to make a new record.
I just did that and results are still the same though.

I think you've fixed this for individual instruments repeating at the loss of entire event support. If we're lucky, there might be a way to detect this.

I didn't realize I had that enabled when testing. When that setting is enabled both event instances and "Repeat entire event" are handled the same.

I think the best option is to determine x in $data[$prev_event][x] on whether the event is an event instance or not. Do you know of a better way than $_GET['instance'] > 1 to check if the current event is an event instance?

Thoughts?

I think that's the easiest way to do it actually. There is a something (I'm blanking on the name, possibly part of the $Proj object) that tracks all repeating forms if the $_GET alone doesn't cut it.

ChemiKyle · 2021-06-29T15:42:05Z

Cross-event population is working again, but cross-instance population is still failing. In cross-instance, the $prev_event is the id for "Event 4".

ChemiKyle · 2021-06-29T15:50:19Z

ExternalModule.php

+                        // Event instances share the same event id for every instance. 
+                        // To support event instances we only break on the first instance, and allow execution for all future instances i.e., $_GET['instance'] greater than 1. 
+                        // Event instances are enabled under: Project Setup > Enable optional modules and customizations > Repeatable instruments and events > 'Repeat instruments (repeat independently of each other)`
+                        if ($event == $_GET['event_id'] && $_GET['instance'] == 1) {


This is the cause of the event-id mismatch in cross-instance population. The bug I'm seeing results from this foreach running to the end of the $events, resulting the final event being set as $prev_event

ChemiKyle · 2021-07-26T15:11:08Z

ExternalModule.php

+     * Returns true if the event is a "Repeat Entire Event (repeat all instruments together)". 
+     */
+    function isRepeatEntireEvent() {
+        if ($_GET['instance'] > 1 && isset($_GET['oldinstance'])) {


Almost there, but still having some inconsistency. When creating the second instance of a repeat event (repeat entire event), the fields don't autopopulate with the prior event. I think this line is the cause

When I create the 2nd instance of an event on a new record there is no oldinstance param, so this condition doesn't fire. The third instance (and probably onward) works as expected.

To recreate

Start a new record, fill in values for events 1 and 2.

Observe the initial instance of event 3 will autopopulate with event 2 info.

Create a new instance of event 3, observe that data does not autopopulate

Fill in some info here, save and create a 3rd instance; observe that the 3rd instance does autopopulate from the 2nd.

mschulze0 · 2021-08-03T18:27:01Z

Would love to test once working :)
I just tried the current version and sometimes it works, sometimes it doesn't.

michael-bentz · 2021-08-03T19:12:26Z

Can you elaborate on:

What doesn't work? Repeating events, repeating forms/instruments?
Which version of REDCap are you using?
Do you have this setting enabled?

The expected behavior with chronological events disabled :

For the first instance of repeating event, the module will populate with values from the latest instance from previous event, it will not look beyond the immediate previous event.
For the 2nd instance and beyond of a repeating event, the module will populate with the values from the previous instance of the same event.
For repeating forms/instruments, the module will populate with values from the closest previous form/instrument.

mschulze0 · 2021-08-03T20:17:14Z

For the 2nd instance and beyond of a repeating event, the module will populate with the values from the previous instance of the same event.

I have the chronological detection disabled. Newest REDCap version. Sometimes it populates from the previous instance of the event, sometimes it doesn't populate anything. Haven't found a pattern. What I did notice though is that when you have filled instances, then some empty instances and then a filled instance, it will populate the empty instance with the most recent instance and not with the filled instance immediately before the emepty instance. This shouldn't be happening.

michael-bentz · 2021-08-03T21:01:25Z

Just to make sure REDCap version 11.2.2?

What you describe:

What I did notice though is that when you have filled instances, then some empty instances and then a filled instance, it will populate the empty instance with the most recent instance and not with the filled instance immediately before the emepty instance. This shouldn't be happening.

is the expected behavior of scenario 2. The instance will only look at the immediate previous saved event, regardless of whether it contains non-empty values or not. Given this scenario:

instance 1 (filled)
instance 2 (empty)
instance 3 (filled)

instance 3 will grab the empty values from instance 2 because it is the previous saved event. It contains values, they just happen to be empty strings or other null values.

mschulze0 · 2021-08-04T13:48:51Z

Just to make sure REDCap version 11.2.2?

yes

is the expected behavior of scenario 2.

in that case it would be good to have an option. for our use case we always want to populate from the last filled instance to the left of the instance to be populated.

The instance will only look at the immediate previous saved event,

that's interesting. that might explain why I'm seeing that it sometimes works and sometimes doesn't SEEM to work.

ChemiKyle · 2021-08-04T14:48:27Z

@mschulze0 it sounds like you want autofill with last spatial filled event/instance, enabling chronological previous event population should give you your expected behavior.

If your responses come in out of order you might be able to hack this "fall through until found" behavior with an auxiliary hidden field, main_field_cache, that uses @DEFAULT-FROM-PREVIOUS-EVENT_1='main_field' @DEFAULT-FROM-PREVIOUS-EVENT_2='main_field_cache', that way you have a "cache" that is immutable by the respondent. Your main_field response field could then use @DEFAULT-FROM-PREVIOUS-EVENT_1='main_field' @DEFAULT_2='main_field_cache'.

ChemiKyle · 2021-08-05T15:14:36Z

ExternalModule.php

-            return true;
-        } 
-        return false;
+        return $GLOBALS['isRepeatingForm'];


@mbentz-uf This causes APF to fail to populate fields for an event that is after a repeating form.

If I revert this change, event 4 populates from the latest instance of event 3. A key bit of information is that I have event 4 set to "Repeat instrument".

mschulze0 · 2021-08-06T17:43:46Z

@mschulze0 it sounds like you want autofill with last spatial filled event/instance, enabling chronological previous event population should give you your expected behavior.

If your responses come in out of order you might be able to hack this "fall through until found" behavior with an auxiliary hidden field, main_field_cache, that uses @DEFAULT-FROM-PREVIOUS-EVENT_1='main_field' @DEFAULT-FROM-PREVIOUS-EVENT_2='main_field_cache', that way you have a "cache" that is immutable by the respondent. Your main_field response field could then use @DEFAULT-FROM-PREVIOUS-EVENT_1='main_field' @DEFAULT_2='main_field_cache'.

hmm this sounds very convoluted for something that should be easy.

ChemiKyle · 2021-08-12T14:24:45Z

ExternalModule.php

+                    // The object returned by $instances = $data['repeat_instances'][$event] changes dependening on if the data is from an event or a form.
+                    // For repeating events, the key for $instances is an empty string ("") i.e., ["": ...]
+                    // For repeating forms, the key for $instances is the form name i.e., ["baseline_data": ...]
+                    $instances = ($data['repeat_instances'][$prev_event][$source_form]) ?? ($data['repeat_instances'][$prev_event][""]);


Interesting, I didn't know about the ?? operator. We'll need to bump the minimum PHP version to 7 in the config.json.

ChemiKyle · 2021-08-12T14:53:08Z

Somewhere in that last commit APF was broken when filling forms in out of order.

To recreate:

Fill in event 1
Fill in event 3
Select event 2

Observe there is no auto population regardless of if chronological event detection is on or not.
git checkout ba01745 should bring this back, and it should be different if chronological event detection is on.

As an aside, make sure that you test surveys as well. Really this module begs for an automated testing suite. This is probably a great module to try out making a test with REDCap Cypress.

michael-bentz · 2021-08-12T15:31:49Z

mschulze0 · 2021-08-20T15:11:30Z

Hi @mbentz-uf @ChemiKyle
I narrowed down the problem to this:

I have repeating events with one instance already created and TWO instruments that use APF
When adding a new event instance and entering and saving an APF-enabled instrument the module works as intended
When then entering the other APF instrument in the new event instance the module doesn't populate the fields from the first instance
the module also doesn't populate anything if the second event instance has already been saved by saving a non-APF instrument first (so: add new event instance, enter a non-APF instrument, save --> neither of the two APF instruments get populated)

mschulze0 · 2021-09-16T14:12:32Z

any news on this?

ChemiKyle · 2021-09-20T15:10:04Z

@mschulze0 Michael is on paternity leave until some time in October, we have a lot of COVID related work keeping us busy, if there is a lull I may have time to polish this but I don't expect much movement until October at the earliest.

mschulze0 · 2021-09-20T16:04:07Z

@ChemiKyle thanks for the update!

Add support for event instances

5fd13de

michael-bentz closed this Jun 29, 2021

michael-bentz reopened this Jun 29, 2021

michael-bentz requested a review from ChemiKyle June 29, 2021 14:15

ChemiKyle reviewed Jun 29, 2021

View reviewed changes

michael-bentz force-pushed the add_event_instance_support branch from 87244a0 to 831ba09 Compare July 1, 2021 17:46

Fix bug breaking 'Repeat Entire Event'

5a1933e

michael-bentz force-pushed the add_event_instance_support branch from 831ba09 to 5a1933e Compare July 1, 2021 17:51

michael-bentz requested a review from ChemiKyle July 14, 2021 13:51

ChemiKyle reviewed Jul 26, 2021

View reviewed changes

Fix edge case for the first instance of repeating events

ba01745

michael-bentz requested a review from ChemiKyle July 26, 2021 20:24

ChemiKyle reviewed Aug 5, 2021

View reviewed changes

michael-bentz force-pushed the add_event_instance_support branch from 79eeb8c to af7260d Compare August 11, 2021 16:30

Simplify logic and fix auto populate bug on form deletion

0661885

michael-bentz force-pushed the add_event_instance_support branch from af7260d to 0661885 Compare August 11, 2021 16:31

ChemiKyle reviewed Aug 12, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for event instances #56

Add support for event instances #56

michael-bentz commented Jun 28, 2021

ChemiKyle commented Jun 29, 2021

michael-bentz commented Jun 29, 2021

ChemiKyle commented Jun 29, 2021

ChemiKyle Jun 29, 2021 •

edited

Loading

michael-bentz Jun 29, 2021

ChemiKyle Jun 29, 2021 •

edited

Loading

michael-bentz Jun 29, 2021

ChemiKyle Jun 29, 2021

ChemiKyle commented Jun 29, 2021 •

edited

Loading

ChemiKyle Jun 29, 2021 •

edited

Loading

ChemiKyle Jul 26, 2021 •

edited

Loading

mschulze0 commented Aug 3, 2021

michael-bentz commented Aug 3, 2021 •

edited

Loading

mschulze0 commented Aug 3, 2021

michael-bentz commented Aug 3, 2021

mschulze0 commented Aug 4, 2021

ChemiKyle commented Aug 4, 2021 •

edited

Loading

ChemiKyle Aug 5, 2021 •

edited

Loading

mschulze0 commented Aug 6, 2021

ChemiKyle Aug 12, 2021

ChemiKyle commented Aug 12, 2021 •

edited

Loading

michael-bentz commented Aug 12, 2021 •

edited

Loading

mschulze0 commented Aug 20, 2021

mschulze0 commented Sep 16, 2021

ChemiKyle commented Sep 20, 2021

mschulze0 commented Sep 20, 2021

Add support for event instances #56

Are you sure you want to change the base?

Add support for event instances #56

Conversation

michael-bentz commented Jun 28, 2021

ChemiKyle commented Jun 29, 2021

michael-bentz commented Jun 29, 2021

ChemiKyle commented Jun 29, 2021

ChemiKyle Jun 29, 2021 • edited Loading

Choose a reason for hiding this comment

michael-bentz Jun 29, 2021

Choose a reason for hiding this comment

ChemiKyle Jun 29, 2021 • edited Loading

Choose a reason for hiding this comment

michael-bentz Jun 29, 2021

Choose a reason for hiding this comment

ChemiKyle Jun 29, 2021

Choose a reason for hiding this comment

ChemiKyle commented Jun 29, 2021 • edited Loading

ChemiKyle Jun 29, 2021 • edited Loading

Choose a reason for hiding this comment

ChemiKyle Jul 26, 2021 • edited Loading

Choose a reason for hiding this comment

mschulze0 commented Aug 3, 2021

michael-bentz commented Aug 3, 2021 • edited Loading

The expected behavior with chronological events disabled :

mschulze0 commented Aug 3, 2021

michael-bentz commented Aug 3, 2021

mschulze0 commented Aug 4, 2021

ChemiKyle commented Aug 4, 2021 • edited Loading

ChemiKyle Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

mschulze0 commented Aug 6, 2021

ChemiKyle Aug 12, 2021

Choose a reason for hiding this comment

ChemiKyle commented Aug 12, 2021 • edited Loading

michael-bentz commented Aug 12, 2021 • edited Loading

mschulze0 commented Aug 20, 2021

mschulze0 commented Sep 16, 2021

ChemiKyle commented Sep 20, 2021

mschulze0 commented Sep 20, 2021

ChemiKyle Jun 29, 2021 •

edited

Loading

ChemiKyle Jun 29, 2021 •

edited

Loading

ChemiKyle commented Jun 29, 2021 •

edited

Loading

ChemiKyle Jun 29, 2021 •

edited

Loading

ChemiKyle Jul 26, 2021 •

edited

Loading

michael-bentz commented Aug 3, 2021 •

edited

Loading

ChemiKyle commented Aug 4, 2021 •

edited

Loading

ChemiKyle Aug 5, 2021 •

edited

Loading

ChemiKyle commented Aug 12, 2021 •

edited

Loading

michael-bentz commented Aug 12, 2021 •

edited

Loading