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.

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: allow adding free text questions without sample solution to practice quizzes and microlearnings #4260

Merged
merged 9 commits into from
Sep 15, 2024

Conversation

sjschlapbach
Copy link
Member

@sjschlapbach sjschlapbach commented Sep 15, 2024

Screenshot 2024-09-15 at 19 19 12

Copy link

aviator-app bot commented Sep 15, 2024

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

Copy link

coderabbitai bot commented Sep 15, 2024

Walkthrough

Walkthrough

The pull request introduces significant changes across multiple components in the application, primarily focusing on restructuring data handling. The previous implementation used separate arrays for various properties related to stack elements, which have now been consolidated into a single elements array of objects. This new structure enhances clarity and maintainability. Additionally, modifications to error handling, rendering logic, validation schemas, and GraphQL queries have been implemented to align with the new data model, ensuring consistency and improved data management throughout the application.

Changes

Files Change Summary
AddStackButton.tsx, StackBlockCreation.tsx Consolidated separate arrays for elementIds, titles, types, and hasSampleSolutions into a single elements array of objects, simplifying data structure and enhancing readability.
StackCreationErrors.tsx Introduced StackErrorElement component to modularize error message rendering, shifting from handling multiple properties to iterating over errors.elements.
StackCreationStep.tsx, GroupActivityStackClues.tsx Updated key generation for components to use unique identifiers from the elements array instead of concatenated IDs, improving rendering performance and avoiding key collisions.
WizardLayout.tsx Modified ElementStackFormValues and ElementStackErrorValues interfaces to replace flat arrays with a nested elements array structure, enhancing data organization.
GroupActivityWizard.tsx, MicroLearningWizard.tsx, PracticeQuizWizard.tsx Updated validation schemas and default values to reflect the new elements array structure, ensuring that each element is validated as an object with specific properties.
submitGroupActivityForm.ts, submitMicrolearningForm.ts, submitPracticeQuizForm.ts Changed processing logic to map over stack.elements instead of stack.elementIds, allowing for direct access to element properties and improving clarity in data handling.
QGetSingleMicroLearning.graphql, QGetSinglePracticeQuiz.graphql Updated GraphQL queries to utilize new fragments that include additional fields, enhancing the data structure returned by these queries.
packages/graphql/src/ops.ts Modified query types for GetSingleMicroLearningQuery and GetSinglePracticeQuizQuery to include new fields related to hasSampleSolution and other element data types.
packages/i18n/messages/de.ts, packages/i18n/messages/en.ts Updated localization strings to clarify descriptions of functionalities related to microlearning and practice quizzes, ensuring consistency in messaging.
FreeTextQuestion.tsx, FTEvaluation.tsx, PracticeQuizPoints.tsx Adjusted components to enhance rendering logic and styling, including conditional rendering for answers and modifications to experience points calculation based on the presence of solutions.

Possibly related PRs


Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e4f7188 and d614af4.

Files selected for processing (2)
  • apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (5 hunks)
  • apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (5 hunks)
Additional context used
Biome
apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx

[error] 138-138: Do not add then to an object.

(lint/suspicious/noThenProperty)

apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx

[error] 149-149: Do not add then to an object.

(lint/suspicious/noThenProperty)

Additional comments not posted (9)
apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (4)

123-141: LGTM!

The changes to the validation schema for the elements array look good:

  • Structuring elements as an array of objects improves clarity and maintainability.
  • Validating the id, title, type, and hasSampleSolution properties ensures data integrity.
  • Making hasSampleSolution conditionally required based on the type is a nice touch to allow flexibility for FreeText elements.
Tools
Biome

[error] 138-138: Do not add then to an object.

(lint/suspicious/noThenProperty)


196-204: Looks good!

The mapping of the elements array to the new object structure is implemented correctly:

  • Each element object is constructed with the expected properties.
  • Parsing the id as an integer ensures the correct data type.
  • Defaulting hasSampleSolution to true is a reasonable approach.

49-49: Nice addition!

Adding ElementType.FreeText to the acceptedTypes array is a good change:

  • It allows users to create free text questions in micro-learning sessions.
  • It aligns with the conditional validation of hasSampleSolution for FreeText elements.

138-138: Skipping the static analysis hint.

The usage of the then property at line 138 is valid in the context of conditional validation using yup.when(). It is not adding then to an arbitrary object, but rather defining the validation schema based on a condition. The static analysis hint is a false positive in this case.

Tools
Biome

[error] 138-138: Do not add then to an object.

(lint/suspicious/noThenProperty)

apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (5)

22-22: LGTM!

The unused import statement has been correctly removed.


50-50: LGTM!

The addition of ElementType.FreeText to the acceptedTypes array correctly enables support for free text questions in practice quizzes, aligning with the PR objective.


134-152: LGTM!

The updated validation schema for the elements array correctly validates each element object and conditionally requires a sample solution based on the element's type. This aligns with the PR objective of allowing free text questions without sample solutions.

The static analysis hint about not adding then to an object is a false positive, as it is a valid part of the Yup validation schema.

Tools
Biome

[error] 149-149: Do not add then to an object.

(lint/suspicious/noThenProperty)


167-167: LGTM!

The addition of the elements property to the formDefaultValues object with an empty array as the default value aligns with the updated validation schema and data structure.


207-215: LGTM!

The updated mapping of stack elements to the new format correctly includes the id, title, type, and hasSampleSolution properties, aligning with the new data structure and validation schema. The id property is correctly parsed as an integer, and the hasSampleSolution property is correctly extracted from the options object with a default value of true.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai generate interesting stats about this repository and render them as a table.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

cypress bot commented Sep 15, 2024

klicker-uzh    Run #2959

Run Properties:  status check passed Passed #2959  •  git commit a2bc492ffe ℹ️: Merge d614af452ea05192353a50b64e44fe927b036e29 into 501e612c50daf833ee114e1c3da9...
Project klicker-uzh
Branch Review PracticeQuizFTQuestions
Run status status check passed Passed #2959
Run duration 08m 57s
Commit git commit a2bc492ffe ℹ️: Merge d614af452ea05192353a50b64e44fe927b036e29 into 501e612c50daf833ee114e1c3da9...
Committer Julius Schlapbach
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 43
View all changes introduced in this branch ↗︎

@sjschlapbach sjschlapbach marked this pull request as ready for review September 15, 2024 16:57
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 501e612 and 07d7480.

Files selected for processing (24)
  • apps/frontend-manage/src/components/sessions/creation/AddStackButton.tsx (4 hunks)
  • apps/frontend-manage/src/components/sessions/creation/StackBlockCreation.tsx (7 hunks)
  • apps/frontend-manage/src/components/sessions/creation/StackCreationErrors.tsx (1 hunks)
  • apps/frontend-manage/src/components/sessions/creation/StackCreationStep.tsx (1 hunks)
  • apps/frontend-manage/src/components/sessions/creation/WizardLayout.tsx (1 hunks)
  • apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityStackClues.tsx (1 hunks)
  • apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityWizard.tsx (3 hunks)
  • apps/frontend-manage/src/components/sessions/creation/groupActivity/submitGroupActivityForm.ts (3 hunks)
  • apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (5 hunks)
  • apps/frontend-manage/src/components/sessions/creation/microLearning/submitMicrolearningForm.ts (1 hunks)
  • apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (5 hunks)
  • apps/frontend-manage/src/components/sessions/creation/practiceQuiz/submitPracticeQuizForm.ts (1 hunks)
  • cypress/cypress/e2e/G-microlearning-workflow.cy.ts (5 hunks)
  • packages/graphql/src/graphql/ops/QGetSingleMicroLearning.graphql (1 hunks)
  • packages/graphql/src/graphql/ops/QGetSinglePracticeQuiz.graphql (1 hunks)
  • packages/graphql/src/ops.ts (2 hunks)
  • packages/graphql/src/public/client.json (1 hunks)
  • packages/graphql/src/public/server.json (1 hunks)
  • packages/graphql/src/services/practiceQuizzes.ts (4 hunks)
  • packages/i18n/messages/de.ts (3 hunks)
  • packages/i18n/messages/en.ts (3 hunks)
  • packages/shared-components/src/FreeTextQuestion.tsx (1 hunks)
  • packages/shared-components/src/evaluation/FTEvaluation.tsx (1 hunks)
  • packages/shared-components/src/evaluation/PracticeQuizPoints.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/shared-components/src/evaluation/PracticeQuizPoints.tsx
Additional context used
Learnings (1)
apps/frontend-manage/src/components/sessions/creation/StackCreationErrors.tsx (1)
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4169
File: apps/frontend-manage/src/components/sessions/creation/StackCreationErrors.tsx:53-59
Timestamp: 2024-08-13T15:13:44.365Z
Learning: In the `StackCreationErrors` component, `flatMap` is used to filter out empty or undefined elements from error arrays before mapping them to list entries.
Biome
apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx

[error] 138-138: Do not add then to an object.

(lint/suspicious/noThenProperty)

apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx

[error] 149-149: Do not add then to an object.

(lint/suspicious/noThenProperty)

Additional comments not posted (55)
packages/graphql/src/graphql/ops/QGetSinglePracticeQuiz.graphql (1)

1-5: Verify that the new fragment provides all the necessary data fields.

The changes to the import statement and the fragment being used align with the PR objective of allowing free text questions without sample solutions. However, please ensure that the new PracticeQuizData fragment provides all the necessary data fields required by the application.

Run the following script to verify the usage of the PracticeQuizData fragment:

packages/graphql/src/graphql/ops/QGetSingleMicroLearning.graphql (1)

1-1: Verify the impact of the fragment change on the application.

The import statement has been updated to reference a different GraphQL fragment, FMicroLearningData, instead of FMicroLearningDataWithoutSolutions. This change suggests a shift in the data structure being utilized in the query, potentially altering the information returned by the getSingleMicroLearning operation.

Please ensure that:

  1. The new fragment provides all the necessary data for the application to function correctly.
  2. The change does not introduce any breaking changes or unexpected behavior in the application.
  3. The naming of the fragment accurately reflects the data it contains.

Run the following script to verify the usage of the FMicroLearningData fragment:

Also applies to: 5-5

Verification successful

These new scripts will help us verify the existence of .graphql files and search for the FMicroLearningData fragment without relying on file type recognition. We'll also perform a broader search to catch potential variations or partial matches of the fragment name.


Verified: Fragment change is consistent and impacts only the specified query.

The change from FMicroLearningDataWithoutSolutions to FMicroLearningData in the QGetSingleMicroLearning.graphql file has been verified. This modification is consistent with the codebase structure and appears to be intentional. The new fragment FMicroLearningData is properly defined and includes more comprehensive data, likely incorporating solution-related information.

Key points:

  1. The FMicroLearningData fragment is correctly defined and used.
  2. The impact is limited to the QGetSingleMicroLearning query.
  3. Other queries, such as QGetMicrolearning, continue to use the WithoutSolutions version.

This change may result in more detailed data being returned for the getSingleMicroLearning operation. Ensure that the consuming components can handle the additional information appropriately.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `FMicroLearningData` fragment in the codebase.

# Test 1: Search for the usage of the `FMicroLearningData` fragment.
# Expect: The fragment should be used in other GraphQL queries or mutations.
rg --type graphql $'FMicroLearningData'

# Test 2: Search for the definition of the `FMicroLearningData` fragment.
# Expect: The fragment should be defined in a `.graphql` file.
fd --type file --extension graphql --exec rg $'fragment FMicroLearningData'

Length of output: 147


Script:

#!/bin/bash
# Description: Verify the existence of .graphql files and search for the FMicroLearningData fragment

# Test 1: Check for the existence of .graphql files
echo "Checking for .graphql files:"
fd -e graphql

# Test 2: Search for the usage of the FMicroLearningData fragment in all files
echo -e "\nSearching for FMicroLearningData fragment usage:"
rg "FMicroLearningData"

# Test 3: Search for the definition of the FMicroLearningData fragment
echo -e "\nSearching for FMicroLearningData fragment definition:"
rg "fragment FMicroLearningData"

# Test 4: Perform a broader search for similar fragment names
echo -e "\nPerforming a broader search for similar fragment names:"
rg "fragment.*MicroLearning"

Length of output: 18274

packages/shared-components/src/evaluation/FTEvaluation.tsx (1)

32-44: LGTM!

The conditional rendering of the answers section is a great improvement. It enhances the component's efficiency by preventing unnecessary rendering of empty sections and improves the user interface by avoiding the display of irrelevant information.

The logic for displaying answers is implemented correctly, with appropriate handling of undefined counts. The JSX structure is clean and encapsulates the answers display within the conditional block.

Overall, these changes enhance the component's performance and user experience without introducing any apparent issues or potential bugs.

apps/frontend-manage/src/components/sessions/creation/StackCreationErrors.tsx (2)

8-25: LGTM!

The StackErrorElement component is a clean and reusable way to render individual error messages. It encapsulates the rendering logic and uses the useTranslations hook to access translations. The component takes an error message and its index as props, making it flexible and maintainable.


34-59: Great refactor!

The refactored StackCreationErrors component enhances the readability and maintainability of the code by:

  • Reducing redundancy and improving the separation of concerns.
  • Using the StackErrorElement component to render each error message conditionally based on its existence, making the code more modular and reusable.
  • Utilizing flatMap to filter out empty or undefined elements before mapping them to list entries, aligning with the learning retrieved from long-term memory.

The new structure streamlines the rendering logic and makes it easier to understand and maintain.

apps/frontend-manage/src/components/sessions/creation/practiceQuiz/submitPracticeQuizForm.ts (1)

44-45: LGTM! The new approach improves clarity and maintainability.

The change from using stack.elementIds to stack.elements for constructing the elements array enhances the clarity of the code by directly utilizing the element objects instead of their IDs. This modification improves the readability and maintainability of the code while maintaining the overall functionality.

apps/frontend-manage/src/components/sessions/creation/microLearning/submitMicrolearningForm.ts (1)

47-48: Verify the impact of the change in the structure of the elements array.

The change in the mapping logic suggests that the stack.elements array now contains element objects instead of just element IDs. This change may impact downstream logic that relies on the structure of this array.

Please verify the following:

  1. Ensure that any code that consumes the elements array is updated to handle the new structure of the array.
  2. Verify that the GraphQL mutation payload expects the elements array to contain objects with elementId and order properties.

Run the following script to verify the usage of the elements array:

apps/frontend-manage/src/components/sessions/creation/groupActivity/submitGroupActivityForm.ts (2)

41-44: LGTM!

The code segment correctly maps over values.stack.elements to create an array of objects with elementId and order properties, aligning with the new data model.


71-74: LGTM!

The code segment correctly maps over values.stack.elements to create an array of objects with elementId and order properties, aligning with the new data model.

packages/shared-components/src/FreeTextQuestion.tsx (1)

76-84: LGTM! The changes align with the PR objective.

The modification to the evaluation prop passed to the PracticeQuizPoints component correctly handles the scenario where no solutions are provided for a free text question. By conditionally setting xpAwarded to null when the solutions array is empty, the code enables the addition of free text questions without sample solutions, as intended by the PR.

The changes are localized and should not introduce any errors or have unintended side effects on other parts of the codebase.

apps/frontend-manage/src/components/sessions/creation/WizardLayout.tsx (2)

42-47: Verify the usage of ElementStackFormValues interface across the codebase.

The changes to the ElementStackFormValues interface improve the organization and clarity of element-related data. The new nested object structure enhances encapsulation and makes the relationships between different properties easier to understand.

Please ensure that all code interacting with this interface has been updated to match the new structure. Run the following script to verify the interface usage:


53-60: Verify the error handling code that uses the ElementStackErrorValues interface.

The modifications to the ElementStackErrorValues interface allow for more flexible error handling and better representation of the data structure. The changes align with the updates made to the ElementStackFormValues interface.

Please ensure that the error handling logic has been updated to handle the new structure of the elements property. Run the following script to verify the interface usage in error handling code:

Verification successful

The changes to ElementStackErrorValues interface have been implemented correctly and are being used appropriately.

The updated ElementStackErrorValues interface in WizardLayout.tsx matches the structure described in the review comment. Related components like StackBlockCreation and StackCreationErrors are using this updated interface, suggesting that they are prepared to handle the new error structure.

While we didn't find explicit error handling code (try/catch blocks), this is not a concern as the interface is primarily used for structuring and displaying error data rather than for exception handling.

  • The changes allow for more flexible error representation in element stacks.
  • Components using this interface appear to be ready to work with the new structure.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `ElementStackErrorValues` interface in error handling code.

# Test: Search for the interface usage in error handling contexts. Expect: No mismatches with the new structure.
rg --type typescript -A 5 $'ElementStackErrorValues' | rg --type typescript $'(catch|error)'

Length of output: 160


Script:

#!/bin/bash
# Description: Verify the presence of TypeScript files and search for ElementStackErrorValues usage

# Test 1: Check for TypeScript files
echo "Checking for TypeScript files:"
fd '\.(ts|tsx)$'

# Test 2: Search for ElementStackErrorValues usage without specifying file type
echo -e "\nSearching for ElementStackErrorValues usage:"
rg 'ElementStackErrorValues' -A 5

# Test 3: Search for error handling contexts related to ElementStackErrorValues
echo -e "\nSearching for error handling contexts related to ElementStackErrorValues:"
rg 'ElementStackErrorValues' -A 10 | rg '(catch|error)'

Length of output: 33923

apps/frontend-manage/src/components/sessions/creation/StackCreationStep.tsx (1)

64-64: LGTM! The new key format is an improvement.

Using the id property of the elements array as the key is a better approach compared to the previous format. It creates a more unique and stable key for each StackBlockCreation component, which aligns with React best practices.

This change can potentially lead to improved performance and maintainability, as React can more efficiently identify and update the components when necessary.

apps/frontend-manage/src/components/sessions/creation/AddStackButton.tsx (5)

34-41: LGTM!

The code segment correctly consolidates the data into a single elements array of objects, mapping the properties from the dropped item. The changes align with the AI-generated summary.


61-68: LGTM!

The code segment correctly maps the selection object to an array of objects called stackElements, using map to directly create the array. The changes align with the AI-generated summary.


73-73: LGTM!

The code segment correctly pushes the stackElements array to the elements property of the object being pushed, aligning with the new data structure.


99-106: LGTM!

The code segment correctly consolidates the data into a single elements array of objects, mapping the properties from each selection object. The changes align with the new data structure.


136-136: LGTM!

The code segment correctly initializes the elements array as an empty array when a new stack is added, aligning with the new data structure.

apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityStackClues.tsx (1)

69-69: LGTM!

The change to the key prop generation logic for the StackBlockCreation component is a good improvement. It enhances the uniqueness of the key by using the actual id of each element in the stack.elements array, reducing the risk of key collisions and potential rendering issues in React.

apps/frontend-manage/src/components/sessions/creation/StackBlockCreation.tsx (7)

76-83: LGTM!

The changes to the drop event handler align with the restructuring of stack elements into a single elements array. The new element is created using the properties of the dropped item and added to the elements array of the stack object.


200-206: LGTM!

The changes to the rendering logic align with the restructuring of stack elements into a single elements array. The map function is used to iterate over the stack.elements array directly, simplifying the rendering logic.


220-221: LGTM!

The change to access the title property directly from the element object aligns with the restructuring of stack elements into a single elements array. It simplifies the rendering logic.


235-243: LGTM!

The changes to the logic for moving an element up within the elements array align with the restructuring of stack elements into a single elements array. The R.move function is used to update the elements array correctly.


259-272: LGTM!

The changes to the logic for moving an element down within the elements array align with the restructuring of stack elements into a single elements array. The R.move function is used to update the elements array correctly.


292-294: LGTM!

The changes to the logic for deleting an element from the elements array align with the restructuring of stack elements into a single elements array. The slice method is used to remove the element at the current questionIdx correctly.


314-324: LGTM!

The changes to the logic for pasting selected questions into the elements array align with the restructuring of stack elements into a single elements array. The selected questions are mapped to new element objects and concatenated with the existing stack.elements array correctly.

apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (4)

20-20: LGTM!

The updated import statement looks good.


49-49: LGTM!

The addition of ElementType.FreeText to the acceptedTypes array is a necessary change to support free text questions without sample solutions in the micro-learning wizard.


123-141: LGTM!

The updated validation schema for the elements array within the stacks array looks good:

  • It ensures that each stack contains at least one element.
  • It validates the structure and properties of each element object.
  • The conditional validation of hasSampleSolution based on the element type aligns with the PR objective of allowing free text questions without sample solutions.

The changes enhance the flexibility and correctness of the validation schema.

Tools
Biome

[error] 138-138: Do not add then to an object.

(lint/suspicious/noThenProperty)


196-204: LGTM!

The mapping of the elements array within each stack of the initialValues looks good:

  • It aligns with the updated data structure for elements.
  • It ensures that the initial form values are consistent with the new structure.
  • The default value of true for hasSampleSolution maintains backward compatibility for existing elements.

The changes are necessary to support the new element structure and ensure proper initialization of the form data.

apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (5)

22-22: LGTM!

The import statement has been updated correctly to align with the changes in the form values structure.


50-50: LGTM!

The addition of ElementType.FreeText to the accepted types aligns with the PR objective of supporting free text questions in practice quizzes.


134-152: Excellent work on restructuring the validation schema!

The new validation schema for the elements array significantly improves the clarity and organization of the data structure. The conditional validation of hasSampleSolution based on the element type is a great addition, ensuring that free text questions do not require a sample solution, which aligns perfectly with the PR objective.

These changes enhance the functionality and flexibility of the practice quiz creation process, allowing for a broader range of question types and more robust validation.

Tools
Biome

[error] 149-149: Do not add then to an object.

(lint/suspicious/noThenProperty)


167-167: LGTM!

The default value for the elements field has been updated correctly to align with the changes in the form values structure.


207-215: Great job on updating the mapping of stack elements!

The refined mapping correctly transforms each element's data into the new format, including id, title, type, and hasSampleSolution. This ensures that the existing stack elements align with the new structure of the form values and validation schema.

These changes maintain consistency throughout the file and contribute to the overall improvement of the practice quiz creation process.

apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityWizard.tsx (3)

124-137: LGTM!

The changes to the validation schema for the stack.elements property align with the restructuring mentioned in the AI-generated summary. The validation ensures that the elements array contains at least one object, and each object has the required properties (id, title, and type). The type validation against the acceptedTypes array enforces the allowed element types.

These changes improve the organization and clarity of the validation schema and help maintain the integrity of the data structure.


181-181: LGTM!

The change to the default value for the stack.elements property aligns with the restructuring mentioned in the AI-generated summary. By initializing elements as an empty array, it ensures consistency with the updated validation schema and provides a proper starting point for the form.


228-235: LGTM!

The changes to the initialization of the stack.elements property when initialValues are provided align with the restructuring mentioned in the AI-generated summary. The mapping creates objects with id, title, type, and hasSampleSolution properties for each element in initialValues.stacks[0].elements.

Setting hasSampleSolution to false for each element is consistent with the PR objective of allowing free text questions without sample solutions.

These changes ensure that the initial values for the stack.elements property are properly mapped to the new data structure.

packages/graphql/src/public/client.json (2)

134-134: Verify the underlying changes to the GetSingleMicroLearning query.

The hash value for the GetSingleMicroLearning query has been modified, indicating changes to its implementation or returned data structure. To better understand the reason and potential impact of this change, please provide more context about the specific modifications made to this query.


135-135: Confirm the GetSinglePracticeQuiz query changes are related to allowing free text questions without sample solutions.

The hash value for the GetSinglePracticeQuiz query has been modified. Based on the PR summary, this change is likely related to supporting the addition of free text questions without sample solutions to practice quizzes.

Can you please confirm that the modifications to this query were made to enable this new feature? It would be helpful to understand how the query and/or its returned data structure were updated to accommodate questions without sample solutions.

cypress/cypress/e2e/G-microlearning-workflow.cy.ts (5)

17-18: LGTM!

The new constants questionTitle2 and question2 are declared correctly using uuid() to ensure uniqueness across test runs. They will likely be used to set up a second SC question without a sample solution later in the test.


41-48: Looks good!

The new test step correctly sets up a second SC question without configuring the sample solution. This aligns with the PR objective and sets up the necessary preconditions to validate allowing questions without sample solutions later in the test.


160-162: Great addition!

The new assertions correctly verify that free text questions without sample solutions are accepted in the microlearning and don't block submission. This aligns perfectly with the PR objective.


189-202: Excellent validation!

The new test step does a great job validating that SC questions without sample solutions are rejected when added to certain stacks, likely based on some predefined criteria. It ensures that such invalid questions disable the submit button until they are removed.

This aligns perfectly with the PR objective and helps maintain the integrity of the microlearning by enforcing necessary validation rules. Well done!


286-287: Spot on!

The changes correctly validate the behavior when a student answers a microlearning containing a free text question without a sample solution.

After answering the SC question, the submit button remains disabled until the free text answer is entered. This aligns perfectly with the expected behavior for free text questions.

Great job covering this student-facing workflow!

packages/graphql/src/services/practiceQuizzes.ts (2)

1148-1150: LGTM!

The change correctly returns the results.responses object in the answers field only if elementData.options.hasSampleSolution is true. When there is no sample solution, an empty object is returned instead, ensuring that actual responses are not exposed.


1862-1864: LGTM!

For 'FREE_TEXT' elements, the change correctly includes the actual elementData.options.solutions array in the returned evaluation object only if elementData.options.hasSampleSolution is true. When there is no sample solution, an empty array is returned instead, ensuring that the actual solutions are not exposed.

packages/i18n/messages/en.ts (3)

1005-1007: LGTM!

The updated description provides more clarity by stating that all available element types can be included in a microlearning. This aligns with the PR objective of allowing free text questions without sample solutions.


1114-1115: LGTM!

The updated message enhances clarity by specifying that the sample solution requirement applies to all element types except free text questions. This aligns perfectly with the PR objective.


1148-1149: LGTM!

The updated description clarifies that practice quizzes can contain all available element types, including free text questions. This change aligns with the PR objective and provides a more inclusive description.

packages/i18n/messages/de.ts (3)

1006-1007: LGTM!

The updated description accurately reflects the expanded scope of elements that can be included in microlearnings.


1119-1121: LGTM!

The refined requirement clearly communicates that model solutions are necessary for all question types except free text questions. This aligns perfectly with the PR objective.


1155-1156: LGTM!

The updated description accurately reflects the expanded scope of element types that can be included in practice quizzes.

packages/graphql/src/ops.ts (2)

3237-3244: LGTM!

The modifications to the GetSingleMicroLearningQuery and GetSinglePracticeQuizQuery types enhance the functionality and data richness of the queries. The addition of the hasSampleSolution field, the correct field for choices, and the solutionRanges for numerical elements provide more detailed information about the elements involved in micro-learnings and practice quizzes.

The changes are consistent across both query types and align with the list of alterations provided in the summary.


3514-3515: Looks good!

The modifications to the GetSingleMicroLearningDocument and GetSinglePracticeQuizDocument GraphQL document definitions are consistent with the changes made to the corresponding query types. The usage of the ElementData fragment ensures that the updated fields are included in the query selection sets.

These changes ensure that the GraphQL queries retrieve the newly added fields from the server, maintaining consistency between the client and server.

Copy link

sonarcloud bot commented Sep 15, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
24.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@sjschlapbach sjschlapbach merged commit 3da722c into v3 Sep 15, 2024
11 of 14 checks passed
@sjschlapbach sjschlapbach deleted the PracticeQuizFTQuestions branch September 15, 2024 19:20
Copy link

cypress bot commented Sep 15, 2024

klicker-uzh    Run #2960

Run Properties:  status check failed Failed #2960  •  git commit 3da722c3e7: feat: allow adding free text questions without sample solution to practice quizz...
Project klicker-uzh
Branch Review v3
Run status status check failed Failed #2960
Run duration 09m 53s
Commit git commit 3da722c3e7: feat: allow adding free text questions without sample solution to practice quizz...
Committer Julius Schlapbach
View all properties for this run ↗︎

Test results
Tests that failed  Failures 1
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 42
View all changes introduced in this branch ↗︎

Tests for review

Failed  cypress/e2e/E-course-workflow.cy.ts • 1 failed test

View Output

Test Artifacts
Test course creation and editing functionalities > Test the assignment of random groups in the seeded test course Test Replay Screenshots
Flakiness  cypress/e2e/F-live-quiz-workflow.cy.ts • 1 flaky test

View Output

Test Artifacts
Different live-quiz workflows > creates a live quiz without questions and tests the feedback mechanisms and deletes the completed live session Test Replay Screenshots

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants