CarnegieLearningWeb / UpGrade

Framework for adding A/B testing to education applications
https://www.upgradeplatform.org/
BSD 3-Clause "New" or "Revised" License
26 stars 13 forks source link

Exclusions are not handled as expected in mark call and code needs optimization #1301

Closed ppratikcr7 closed 3 weeks ago

ppratikcr7 commented 8 months ago

Version where bug was found: e.g "5.1.0"

Describe the bug Currently the various exclusions codes are not stored in the DB for simple and within subject experiments. Mark call code for exclusions needs optimization to not call various code blocks for all experiment state. We are also storing multiple rows for Individual Enrollments for within subject experiments and we are also storing multiple rows for monitored decision point table for experiment level exclusions.

To Reproduce Steps to reproduce the behavior for few exclusions, or refer to the detailed document for all exclusion codes:

  1. Try to exclude a user who is on exclusion list using a segment or individual exclusion list.
  2. Once you hit /mark api for the user, you wont see its Data as the table is not getting populated with the reason for exclusion.

Expected behavior Exclusions should work as expected for the above scenario.

bcb37 commented 7 months ago

@ppratikcr7 After testing this, I have a question about the behavior outlined in the linked document:

If a participant is excluded because their group has been excluded, a record is added to the individual_exclusion table with PARTICIPANT_ON_EXCLUSION_LIST code, in addition to a record being added to the group_exclusion table with GROUP_ON_EXCLUSION_LIST code. Do we want both of those records created in that case?

Also, if a student with no excluded groups enrolls with a given working group, and then another student enrolls with the same working group, but with an already excluded group in their group list, that working group will be excluded. However, any previous enrollments from that working group are still there. Is this a concern? (The group enrollment is indeed removed, but the individual enrollments that would have been excluded are still there)

VivekFitkariwala commented 5 months ago

In case of individual consistency, we need a record in that table.

The second one is a concern. This is similar to the discussion we had on this ticket