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

add option fields to lock answer group #518

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

sharlotta93
Copy link
Contributor

@sharlotta93 sharlotta93 commented Nov 13, 2024

PR Type

enhancement


Description

  • Added a new optional boolean field lockFormAnswerGroup to the configuration files for both single and multiple form responses.
  • Updated the validation schema to accommodate the new lockFormAnswerGroup field.
  • Implemented logic in both pushFormResponseToHealthie.ts and pushFormResponsesToHealthie.ts to handle the locking of form answer groups based on the new field.
  • Utilized lodash's defaultTo function to manage default values for the lock option.
  • Added conditional mutations to lock the form answer group if the lockFormAnswerGroup field is set to true.

Changes walkthrough 📝

Relevant files
Enhancement
fields.ts
Add lock option to form response fields configuration       

extensions/healthie/actions/dataExchange/pushFormResponseToHealthie/config/fields.ts

  • Added a new field lockFormAnswerGroup to the fields configuration.
  • Updated the validation schema to include lockFormAnswerGroup.
  • +8/-0     
    pushFormResponseToHealthie.ts
    Implement form locking logic in pushFormResponseToHealthie

    extensions/healthie/actions/dataExchange/pushFormResponseToHealthie/pushFormResponseToHealthie.ts

  • Imported defaultTo from lodash.
  • Added logic to handle the locking of form answer groups.
  • Included a conditional mutation to lock the form if required.
  • +20/-1   
    fields.ts
    Add lock option to form responses fields configuration     

    extensions/healthie/actions/dataExchange/pushFormResponsesToHealthie/config/fields.ts

  • Added a new field lockFormAnswerGroup to the fields configuration.
  • Updated the validation schema to include lockFormAnswerGroup.
  • +8/-0     
    pushFormResponsesToHealthie.ts
    Implement form locking logic in pushFormResponsesToHealthie

    extensions/healthie/actions/dataExchange/pushFormResponsesToHealthie/pushFormResponsesToHealthie.ts

  • Imported defaultTo from lodash.
  • Added logic to handle the locking of form answer groups.
  • Included a conditional mutation to lock the form if required.
  • +19/-1   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Field Definition
    The new field lockFormAnswerGroup is added with appropriate type and description. Ensure that the field is correctly utilized in all relevant parts of the application.

    Lock Logic
    The locking mechanism based on lockFormAnswerGroup is implemented. Review the conditional logic and ensure it aligns with business requirements.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Implement error handling for the form locking process to enhance reliability

    Add error handling for the lockFormAnswerGroup mutation to manage potential failures
    during the locking process.

    extensions/healthie/actions/dataExchange/pushFormResponseToHealthie/pushFormResponseToHealthie.ts [91-101]

    -await healthieSdk.client.mutation({
    -  lockFormAnswerGroup: {
    -    __args: {
    -      input: {
    -        id: fields.healthieFormId,
    +try {
    +  await healthieSdk.client.mutation({
    +    lockFormAnswerGroup: {
    +      __args: {
    +        input: {
    +          id: fields.healthieFormId,
    +        },
    +      },
    +      form_answer_group: {
    +        id: true,
           },
         },
    -    form_answer_group: {
    -      id: true,
    -    },
    -  },
    -})
    +  })
    +} catch (error) {
    +  console.error('Failed to lock the form answer group:', error);
    +}
    Suggestion importance[1-10]: 8

    Why: Adding error handling around the mutation operation is crucial to manage potential failures and ensure the robustness of the application. This suggestion correctly identifies a missing aspect in the error management of the PR.

    8
    Best practice
    Validate the success of the form locking operation to ensure data integrity

    Consider checking the response of the lockFormAnswerGroup mutation to confirm that
    the form was successfully locked.

    extensions/healthie/actions/dataExchange/pushFormResponseToHealthie/pushFormResponseToHealthie.ts [91-101]

    -await healthieSdk.client.mutation({
    +const lockResponse = await healthieSdk.client.mutation({
       lockFormAnswerGroup: {
         __args: {
           input: {
             id: fields.healthieFormId,
           },
         },
         form_answer_group: {
           id: true,
         },
       },
    -})
    +});
    +if (!lockResponse) {
    +  throw new Error('Failed to lock the form answer group');
    +}
    Suggestion importance[1-10]: 7

    Why: Checking the response of the mutation to confirm successful execution is a best practice that enhances data integrity and reliability. This suggestion is relevant and improves the current implementation by ensuring the lock operation's success.

    7
    Maintainability
    Improve code organization and maintainability by isolating the form locking logic

    Refactor the locking logic into a separate function to improve code modularity and
    reuse.

    extensions/healthie/actions/dataExchange/pushFormResponseToHealthie/pushFormResponseToHealthie.ts [90-102]

    -if (lock) {
    -  await healthieSdk.client.mutation({
    +async function lockFormAnswerGroup(id) {
    +  return await healthieSdk.client.mutation({
         lockFormAnswerGroup: {
           __args: {
             input: {
    -          id: fields.healthieFormId,
    +          id: id,
             },
           },
           form_answer_group: {
             id: true,
           },
         },
    -  })
    +  });
    +}
    +if (lock) {
    +  const lockResult = await lockFormAnswerGroup(fields.healthieFormId);
    +  if (!lockResult) {
    +    throw new Error('Failed to lock the form answer group');
    +  }
     }
    Suggestion importance[1-10]: 6

    Why: Refactoring the locking logic into a separate function improves modularity and maintainability. This suggestion enhances code organization and potentially facilitates reuse in other parts of the application.

    6

    Copy link
    Contributor

    @mohsinht mohsinht left a comment

    Choose a reason for hiding this comment

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

    Looks good! I see that freeze didn't work, did you remove it completely from the changes?

    @sharlotta93 sharlotta93 merged commit 64dbdaa into main Nov 13, 2024
    2 of 3 checks passed
    @sharlotta93 sharlotta93 deleted the ET-355-add-lock-attribute-to-healthie branch November 13, 2024 09:43
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants