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

Update create user and languages #171

Closed
wants to merge 2 commits into from

Conversation

jamalsoueidan
Copy link
Owner

@jamalsoueidan jamalsoueidan commented Jul 3, 2024

PR Type

Enhancement, Tests


Description

  • Refactored various user-related fields by removing professions, aboutMe, aboutMeHtml, and shortDescription.
  • Renamed speaks field to languages across multiple files.
  • Added username field to user schema and availability generation test.
  • Updated tests to reflect changes in user fields.

Changes walkthrough 📝

Relevant files
Enhancement
9 files
create.ts
Refactor customer creation schema fields.                               

src/functions/customer/controllers/customer/create.ts

  • Removed professions, aboutMe, aboutMeHtml, and shortDescription
    fields.
  • Renamed speaks field to languages.
  • +1/-5     
    create-article.ts
    Update tag creation logic for languages field.                     

    src/functions/customer/orchestrations/customer/create/create-article.ts

    • Renamed speaks field to languages in tag creation logic.
    +2/-2     
    create-user-metaobject.ts
    Update user metaobject creation logic for languages field.

    src/functions/customer/orchestrations/customer/create/create-user-metaobject.ts

  • Renamed speaks field to languages in user metaobject creation logic.
  • +2/-2     
    update-article.ts
    Update tag update logic for languages field.                         

    src/functions/customer/orchestrations/customer/update/update-article.ts

    • Renamed speaks field to languages in tag update logic.
    +2/-2     
    update-user-metaobject.ts
    Update user metaobject update logic for languages field. 

    src/functions/customer/orchestrations/customer/update/update-user-metaobject.ts

  • Renamed speaks field to languages in user metaobject update logic.
  • +2/-2     
    create.ts
    Refactor customer service creation fields.                             

    src/functions/customer/services/customer/create.ts

  • Removed professions, aboutMe, aboutMeHtml, and shortDescription
    fields.
  • Renamed speaks field to languages.
  • +1/-9     
    search.ts
    Update user search logic for languages field.                       

    src/functions/user/services/user/search.ts

    • Renamed speaks field to languages in user search logic.
    +2/-2     
    user.schema.ts
    Update user schema for languages field and username constraint.

    src/functions/user/user.schema.ts

  • Added required constraint to username field.
  • Renamed speaks field to languages.
  • +3/-3     
    user.types.ts
    Update user type definition for languages field.                 

    src/functions/user/user.types.ts

    • Renamed speaks field to languages in user type definition.
    +1/-1     
    Tests
    4 files
    create-user-metaobject.spec.ts
    Update user metaobject creation test for languages field.

    src/functions/customer/orchestrations/customer/create/create-user-metaobject.spec.ts

  • Renamed speaks field to languages in user metaobject creation test.
  • +3/-3     
    update-user-metaobject.spec.ts
    Update user metaobject update test for languages field.   

    src/functions/customer/orchestrations/customer/update/update-user-metaobject.spec.ts

  • Renamed speaks field to languages in user metaobject update test.
  • +3/-3     
    generate-availability.spec.ts
    Update availability generation test for username field.   

    src/library/availability/generate-availability.spec.ts

  • Added username field to user creation in availability generation test.

  • +2/-1     
    user.ts
    Update user object helper for languages field.                     

    src/library/jest/helpers/user.ts

    • Renamed speaks field to languages in user object helper.
    +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    github-actions bot commented Jul 3, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests Yes
    🔒 Security concerns No
    ⚡ Key issues to review Field Renaming Consistency:
    The field speaks has been renamed to languages in multiple files. Ensure that all references to the old field name are updated across the entire codebase to maintain consistency and prevent runtime errors.
    Data Migration:
    With the removal of fields like professions, aboutMe, aboutMeHtml, and shortDescription, ensure that there is a plan or script for migrating existing data that may rely on these fields. This is crucial to prevent data loss and application errors post-deployment.

    Copy link

    github-actions bot commented Jul 3, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Improve the formatting of language tags to ensure each language is correctly prefixed

    Ensure that the tags are correctly formatted by separating each language with a comma,
    rather than concatenating all languages with a prefix repeatedly.

    src/functions/customer/orchestrations/customer/create/create-article.ts [23]

     if (user.languages) {
    -  tags.push(`language-${user.languages.join(", language-")}`);
    +  tags.push(...user.languages.map(lang => `language-${lang}`));
     }
     
    Suggestion importance[1-10]: 9

    Why: The suggestion improves the formatting of language tags, ensuring each language is correctly prefixed, which enhances readability and correctness.

    9
    Enhancement
    Enhance the robustness of the languages attribute check by using nullish coalescing

    Use a more robust check for the presence of languages to handle potential null or
    undefined values effectively.

    src/functions/customer/orchestrations/customer/create/create-user-metaobject.ts [42]

    -value: JSON.stringify({ languages: user.languages || [] }),
    +value: JSON.stringify({ languages: user.languages ?? [] }),
     
    Suggestion importance[1-10]: 8

    Why: Using nullish coalescing is a more robust way to handle potential null or undefined values, improving code reliability.

    8
    Maintainability
    Enforce the fullname field as required to maintain data integrity

    Ensure that the fullname field is consistently required across the schema to prevent
    inconsistencies.

    src/functions/user/user.schema.ts [48]

    -fullname: String, //will be updated by shopify webhook
    +fullname: {
    +  type: String,
    +  required: true
    +}, //will be updated by shopify webhook
     
    Suggestion importance[1-10]: 8

    Why: Making the fullname field required ensures data consistency and integrity across the schema, which is important for maintainability.

    8
    Best practice
    Add error handling to the customer creation service to improve robustness

    Include error handling for the async function to manage exceptions and maintain
    robustness.

    src/functions/customer/services/customer/create.ts [8]

    -export const CustomerServiceCreate = async (
    +export const CustomerServiceCreate = async () => {
    +  try {
    +    // existing implementation
    +  } catch (error) {
    +    console.error("Failed to create customer:", error);
    +    throw error;
    +  }
    +}
     
    Suggestion importance[1-10]: 7

    Why: Adding error handling is a best practice that improves the robustness of the async function, but the suggestion does not provide the exact context of the existing implementation.

    7

    @jamalsoueidan
    Copy link
    Owner Author

    We wait with this changes.

    @jamalsoueidan jamalsoueidan deleted the update-create-user-and-languages branch July 3, 2024 01:10
    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.

    1 participant