feat: Integrated Optimizely Prompt Experiment #52

rijuma · 2024-06-26T17:34:56Z

This integrates Optimizely's hooks for the new XPert prompt experiment.

varshamenon4

Nice! Just a few questions.

varshamenon4 · 2024-06-27T18:23:15Z

src/constants/experiments.js

@@ -0,0 +1,7 @@
+const PROMPT_EXPERIMENT_FLAG = '_cosmo__xpert_gpt_4_0_prompt';


Question: sorry if this is captured somewhere else already but is the naming of this flag based on what is set in optimizely?

Yes. It's taken from here:

that looks like the correct value to use

varshamenon4 · 2024-06-27T18:24:29Z

src/data/api.js

+    unitId,
+    ...customQueryParams,
+  };
+


varshamenon4 · 2024-06-27T18:28:15Z

src/utils/index.jsx

Question: Should this file exist or was it deleted?

It was empty. I don't think it should be here.

* feat: trigger survey monkey on chat close * fix: Fixed useOptimizelyExperiment() infinite updates * fix: read experiment state correctly * feat: add tests * fix: fix lint errors * fix: remove flaky test --------- Co-authored-by: Marcos <[email protected]>

rijuma · 2024-07-10T12:42:38Z

package.json

@@ -19,7 +19,8 @@
    "lint:fix": "fedx-scripts eslint --fix --ext .js --ext .jsx .",
    "snapshot": "fedx-scripts jest --updateSnapshot",
    "start": "fedx-scripts webpack-dev-server --progress",
-    "test": "fedx-scripts jest --coverage --passWithNoTests"
+    "test": "fedx-scripts jest --coverage --passWithNoTests",
+    "test:ci": "fedx-scripts jest --silent --coverage --passWithNoTests"


This reduces CI checks log file size from 30KB to 8KB by silencing warnings.
At some point warnings should be reduced, but we are not taking them into account on the CI, so we just might just remove them.

Also, some warnings are caused by external libraries, like @openedx/paragon icons for example. When testing, the library renders the icons as <IconMock /> by the use of a string reference rather than a component causing the following warning:

Warning: <IconMock /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements. at IconMock at button at /Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/react-bootstrap/cjs/Button.js:28:23 at children (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/@openedx/paragon/src/Button/index.jsx:12:3) at div at isOpen (/Users/marcos/edx/src/frontend-lib-learning-assistant/src/components/ToggleXpertButton/index.jsx:20:3) at Provider (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/react-redux/lib/components/Provider.js:21:20) at IntlProvider (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/react-intl/src/components/provider.js:33:47) at children (/Users/marcos/edx/src/frontend-lib-learning-assistant/src/utils/utils.test.jsx:19:22)

This warning stacks up for every render.

rijuma · 2024-07-11T17:09:23Z

src/components/Disclosure/index.jsx

+    {getConfig().PRIVACY_POLICY_URL ? (
+      <p className="disclaimer text-light-100 py-3">
+        <strong>Note: </strong>
+        This chat is AI generated (powered by ChatGPT). Mistakes are possible.
+        By using it you agree that edX may create a record of this chat.
+        Your personal data will be used as described in our&nbsp;
+        <Hyperlink
+          className="privacy-policy-link text-light-100"
+          destination={getConfig().PRIVACY_POLICY_URL}
+        >
+          privacy policy
+        </Hyperlink>
+        .
+      </p>
+    ) : null }


This solves a warning in the tests. Since getConfig().PRIVACY_POLICY_URL is undefined on the test environment, it throws a warning since destination is a required prop for <Hyperlink>.

The full warning was:

Warning: Failed prop type: The prop `destination` is marked as required in `ForwardRef`, but its value is `undefined`. at className (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/@openedx/paragon/src/Hyperlink/index.jsx:15:5) at _classCallCheck (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/@openedx/paragon/src/withDeprecatedProps.tsx:28:27) at p at div at children (/Users/marcos/edx/src/frontend-lib-learning-assistant/src/components/Disclosure/index.jsx:10:23) at div at courseId (/Users/marcos/edx/src/frontend-lib-learning-assistant/src/components/Sidebar/index.jsx:23:3) at Provider (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/react-redux/lib/components/Provider.js:21:20) at IntlProvider (/Users/marcos/edx/src/frontend-lib-learning-assistant/node_modules/react-intl/src/components/provider.js:33:47) at children (/Users/marcos/edx/src/frontend-lib-learning-assistant/src/utils/utils.test.jsx:19:22)

since it seems like this config value is effectively required for this component maybe we want to make use of ensureConfig()? https://github.com/openedx/frontend-platform/blob/b0ac45329f0ed9022541abea4fe30abf49fde0b1/src/config.js#L264

Indeed. I wasn't familiar with all the config options. Updated and it's much cleaner now. Thanks for the suggestion.

zacharis278 · 2024-07-11T17:36:15Z

src/components/Sidebar/index.test.jsx

@@ -0,0 +1,160 @@
+import React from 'react';


this is great! Thanks for improving our test coverage on this

src/hooks/useOptimizelyExperiment.js

src/utils/index.jsx

Co-authored-by: Zachary Hancock <[email protected]>

This reverts commit e9791c8.

rijuma force-pushed the rijuma/optimizely-prompt-experiment branch from 10c5989 to 6c5aeb1 Compare June 27, 2024 14:56

feat: Integrated Optimizely Prompt Experiment

2fb1862

rijuma force-pushed the rijuma/optimizely-prompt-experiment branch from 6c5aeb1 to 2fb1862 Compare June 27, 2024 14:57

varshamenon4 reviewed Jun 27, 2024

View reviewed changes

rijuma and others added 5 commits July 1, 2024 18:07

fix: Fixed useOptimizelyExperiment() infinite updates

3412f8f

chore: Added unit tests to useOptimizelyExperiment()

62aaec8

chore: Updated tests to silence warnings on CI

58691b9

chore: Added coverage for experiment on Sidebar

bd16de8

rijuma commented Jul 10, 2024

View reviewed changes

fix: Updated Sidebar test and fixed warning

8d96749

rijuma force-pushed the rijuma/optimizely-prompt-experiment branch from f4da8a6 to 8d96749 Compare July 11, 2024 14:04

rijuma added 2 commits July 11, 2024 13:54

chore: Added test coverage for ToggleXpertButton component

7e6a5a1

chore: Updated logic for privacy policy in Dixclosure

7867ea2

rijuma commented Jul 11, 2024

View reviewed changes

rijuma marked this pull request as ready for review July 11, 2024 17:12

zacharis278 reviewed Jul 11, 2024

View reviewed changes

src/hooks/useOptimizelyExperiment.js Outdated Show resolved Hide resolved

zacharis278 reviewed Jul 11, 2024

View reviewed changes

src/utils/index.jsx Outdated Show resolved Hide resolved

rijuma and others added 3 commits July 11, 2024 15:51

fix: Added ensureConfig() rather than conditioning the render

cb402b8

fix: Casting number to string for useDecision() payload.

c055843

Co-authored-by: Zachary Hancock <[email protected]>

Merge branch 'main' into rijuma/optimizely-prompt-experiment

03ce47e

zacharis278 approved these changes Jul 11, 2024

View reviewed changes

chore: Updated test based on Optimizely fix

03b500a

rijuma merged commit e9791c8 into main Jul 11, 2024
5 checks passed

rijuma deleted the rijuma/optimizely-prompt-experiment branch July 11, 2024 19:04

This was referenced Jul 15, 2024

chore: Update learning assistant lib openedx/frontend-app-learning#1419

Merged

chore: Revert Learning Assistant upgrade due to a bug openedx/frontend-app-learning#1420

Merged

rijuma added a commit that referenced this pull request Jul 15, 2024

Revert "feat: Integrated Optimizely Prompt Experiment (#52)"

5a6c343

This reverts commit e9791c8.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Integrated Optimizely Prompt Experiment #52

feat: Integrated Optimizely Prompt Experiment #52

rijuma commented Jun 26, 2024

varshamenon4 left a comment

varshamenon4 Jun 27, 2024

rijuma Jul 3, 2024

zacharis278 Jul 11, 2024

varshamenon4 Jun 27, 2024

varshamenon4 Jun 27, 2024

rijuma Jul 1, 2024

rijuma Jul 10, 2024

rijuma Jul 11, 2024 •

edited

Loading

rijuma Jul 11, 2024 •

edited

Loading

zacharis278 Jul 11, 2024

rijuma Jul 11, 2024

zacharis278 Jul 11, 2024

		@@ -0,0 +1,7 @@
		const PROMPT_EXPERIMENT_FLAG = '_cosmo__xpert_gpt_4_0_prompt';

feat: Integrated Optimizely Prompt Experiment #52

feat: Integrated Optimizely Prompt Experiment #52

Conversation

rijuma commented Jun 26, 2024

varshamenon4 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rijuma Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

rijuma Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rijuma Jul 11, 2024 •

edited

Loading

rijuma Jul 11, 2024 •

edited

Loading