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

Return an empty object for experiment point/ID where user is getting "default" #154

Closed nirmalpatel closed 2 years ago

nirmalpatel commented 3 years ago

UpGrade never returns "default" as the condition for any user. UpGrade's way of saying that this user is in the "default" condition is to not return anything.

Right now, we don't return an object for the experiment point/ID if the user is getting default. This is causing issues on the client-side where the code cannot iterate over the returned values. To get around this, we will return an empty stub-like object for a point/ID (when the getExperimentCondition()) is called which indicates that the user got the "default" condition.

amurphy-cl commented 2 years ago

@nirmalpatel Where are we on this issue? Please add it to the kanban board if in-progress

YashilDepani commented 2 years ago

Currently the object that is returned ( for non default condition is like this).

[
    {
        "expId": "i",
        "expPoint": "p",
        "twoCharacterId": "QA",
        "assignedCondition": {
            "createdAt": "2022-01-04T14:26:32.137Z",
            "updatedAt": "2022-01-04T14:26:32.137Z",
            "versionNumber": 1,
            "id": "cfaa167e-b461-4e78-8992-26bc53c0b983",
            "twoCharacterId": "EE",
            "name": "",
            "description": null,
            "conditionCode": "c2",
            "assignmentWeight": 50,
            "order": 2
        }
    }
] 

For default condition currently we do not return anything and now instead we would like to return empty object. So we can have two scenarios. Let me know which is the best case:-

Scenario-1

[
   {
        "expId": "i",
        "expPoint": "p",
        "twoCharacterId": "EL"
    }
] 

Scenario-2

[
   {
        "expId": "i",
        "expPoint": "p",
        "twoCharacterId": "EL",
        "assignedCondition": {
            "conditionCode": null
        }
    }
] 

Currently in updated code I am returning scenario-2.

jerith666 commented 2 years ago

I don't think I care too much, as long as both client libraries (TS and Java) can describe this difference in their types such that users of the client libs are forced to account for this.

amurphy-cl commented 2 years ago

Did we decide on a scenario and do we have a PR for this?

VivekFitkariwala commented 2 years ago

I don't know if we really need this change because getAllExperimentCondition should not be directly used by client they should use getExperimentCondition and have we tested this change with clientLibs if they are compatible?