Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: feedback analytics and previously submitted answers #3948

Merged
merged 7 commits into from
Nov 19, 2024
Merged

feat: feedback analytics and previously submitted answers #3948

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24a074e
try again
jamdelion Nov 13, 2024
6896569
Handle previously submitted answers
jamdelion Nov 14, 2024
55ef0bd
Expose feedbackScore separately
jamdelion Nov 14, 2024
ab2f1fc
Lint fix
jamdelion Nov 14, 2024
45ea823
Remove feedback.feedbackscore from allow_lists and add usercomment in…
jamdelion Nov 18, 2024
d80bd2b
Only include feedback_score in analytics tables
jamdelion Nov 19, 2024
04ba77e
Fix sql - readd application.type
jamdelion Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions api.planx.uk/modules/webhooks/service/analyzeSessions/operations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { gql } from "graphql-request";

import { Passport } from "@opensystemslab/planx-core";

import { $api } from "../../../../client/index.js";
import type { Operation } from "../sanitiseApplicationData/types.js";

Expand All @@ -20,6 +21,8 @@ const ALLOW_LIST = [
"application.information.sensitive",
"application.type",
"drawBoundary.action",
"_feedback",
"_feedback.feedbackScore",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure whether _feedback.feedbackScore is needed here, and if it is, whether it is in the right format... The sql file does not have _feedback.feedbackScore in it directly, but seemed to work as it is!

e.g. this is from local hasura, analytics table:
Screenshot 2024-11-14 at 17 06 11

Copy link
Member

@jessicamcinchak jessicamcinchak Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for confusion here! Here's what I'd expect:

  • only "_feedback" should be included in this list. Dot-separated variables only work when it's the "whole" property name and doesn't require destructuring a passport value, which "_feedback.feedbackScore" would
  • This means that feedback column in the raw analytics table will look exactly like you've posted above - a jsonb object !
  • then in the SQL to generate the view you should be able to do something like ((ls.allow_list_answers -> '_feedback' -> 'feedbackScore'::int))::int AS feedback_score so that what we're exposing to Metabase is the feedback score as an integer (Metabase can't process jsonb! And free-text userComments are nice to track in raw table but aren't very chart-able)

Does this make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jess, the int version you put above (and variations I have tried) caused syntax errors "message": "invalid input syntax for type integer: \"feedbackScore\"", - I assume the reason it shouldn't be text is because Metabase can make nice charts with numbers and can't convert from string numbers to actual numbers?

Have added a column for userComment in the table, and I assume that the feedback column (displaying jsonb) is completely redundant then, given that we now have separate columns for feedback_score and user_comment. Figuring out a way to drop that column from the view if so!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-11-18 at 21 02 44

"findProperty.action",
"_overrides",
"planningConstraints.action",
Expand Down Expand Up @@ -61,7 +64,7 @@ export const trackAllowListAnswers: Operation = async () => {

const id = await updateLowcalSessionAllowListAnswers(
sessionId,
allowListAnswers,
allowListAnswers
);
if (id) updatedSessionIds.push(id);
}
Expand Down Expand Up @@ -101,7 +104,7 @@ export const getSubmittedUnAnalyzedSessionIds = async (): Promise<string[]> => {
*/
export const updateLowcalSessionAllowListAnswers = async (
sessionId: string,
allowListAnswers: Passport["data"],
allowListAnswers: Passport["data"]
): Promise<string> => {
try {
const mutation = gql`
Expand All @@ -124,7 +127,7 @@ export const updateLowcalSessionAllowListAnswers = async (
return id;
} catch (error) {
throw new Error(
`Error updating allow_list_answers for lowcal_session ${sessionId}`,
`Error updating allow_list_answers for lowcal_session ${sessionId}`
);
}
};
4 changes: 2 additions & 2 deletions editor.planx.uk/src/@planx/components/Feedback/Public/Public.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import ErrorWrapper from "ui/shared/ErrorWrapper";
import Input from "ui/shared/Input/Input";
import ReactMarkdownOrHtml from "ui/shared/ReactMarkdownOrHtml/ReactMarkdownOrHtml";

import { getPreviouslySubmittedData, makeData } from "../../shared/utils";
import { makeData } from "../../shared/utils";
import { FaceBox } from "../components/FaceBox";
import { createFeedbackSchema, Feedback, FormProps } from "../model";
import { StyledToggleButtonGroup } from "../styled";
Expand Down Expand Up @@ -50,7 +50,7 @@ const FeedbackComponent = (props: PublicProps<Feedback>): FCReturn => {
};

const formik = useFormik<FormProps>({
initialValues: getPreviouslySubmittedData(props) ?? {
initialValues: props.previouslySubmittedData?.data?._feedback ?? {
feedbackScore: "",
userComment: "",
},
Expand Down
31 changes: 31 additions & 0 deletions editor.planx.uk/src/@planx/components/Feedback/Public/tests/Public.submit.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,34 @@ describe("when feedback is required but the user does not submit any data", asyn
});
});
});

describe("When the user presses to go back to the feedback component", () => {
it("recovers the previously submitted answers", async () => {
const handleSubmit = vi.fn();

const { user } = setup(
<FeedbackComponent
handleSubmit={handleSubmit}
feedbackRequired={false}
previouslySubmittedData={{
data: {
_feedback: {
userComment: "I wrote this previously",
feedbackScore: 2,
},
},
}}
/>,
);

expect(screen.getByText("I wrote this previously")).toBeVisible();

await user.click(screen.getByTestId("continue-button"));

expect(insertFeedbackMutation).toHaveBeenCalledWith({
feedbackScore: 2,
feedbackType: "component",
userComment: "I wrote this previously",
});
});
});
3 changes: 3 additions & 0 deletions editor.planx.uk/src/pages/FlowEditor/lib/analytics/provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { DocumentNode } from "@apollo/client";
import { ComponentType as TYPES } from "@opensystemslab/planx-core/types";
import { PASSPORT_FEEDBACK_KEY } from "@planx/components/Feedback/Public/Public";
import Bowser from "bowser";
import { publicClient } from "lib/graphql";
import React, { createContext, useContext, useEffect } from "react";
Expand Down Expand Up @@ -48,6 +49,8 @@ export const ALLOW_LIST = [
"application.information.sensitive",
"application.type",
"drawBoundary.action",
PASSPORT_FEEDBACK_KEY,
"_feedback.feedbackScore",
"findProperty.action",
"_overrides",
"planningConstraints.action",
Expand Down
Loading
Loading