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

fixes and improvements for transaction handling in member and org merge #1673

Merged

Conversation

themarolt
Copy link
Contributor

@themarolt themarolt commented Oct 13, 2023

Changes proposed ✍️

What

🤖 Generated by Copilot at 1d24700

This pull request refactors the transaction handling in several database repository classes and service classes. It simplifies the code by passing the transaction object as part of the options parameter and using a helper method to create consistent repository options. It also improves the error handling and logging of some methods and fixes some minor code style and formatting issues.

🤖 Generated by Copilot at 1d24700

Sing, O Muse, of the skillful refactorings
That the wise developers wrought with their code
To simplify transactions and queries
In the repositories and services of old.

Why

How

🤖 Generated by Copilot at 1d24700

  • Refactor transaction handling in repository and service classes by using repository options instead of transaction parameters (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Add a new static method createTransactionalRepositoryOptions in SequelizeRepository to simplify transaction creation and return a repository options instance with the transaction included (link)
  • Add type annotation for the options parameter in createTransaction method in SequelizeRepository for better type checking and documentation (link)
  • Add MemberRepository and OrganizationRepository prefixes to some static method calls for consistency and clarity (link, link)
  • Remove unused imports and reorder some imports in OrganizationService for code formatting, readability, and consistency (link, link, link)
  • Add try-catch blocks around some calls to triggerMemberSync, triggerRemoveMember, and triggerOrganizationSync to improve error handling and logging in MemberService and OrganizationService (link, link, link)
  • Add checks for the existence of the transaction before rolling it back in MemberService and OrganizationService to prevent errors if the transaction was not created successfully (link, link)
  • Remove some empty lines in merge method in MemberService for code formatting and readability (link, link)

Checklist ✅

  • Label appropriately with Feature, Improvement, or Bug.
  • Add screehshots to the PR description for relevant FE changes
  • New backend functionality has been unit-tested.
  • API documentation has been updated (if necessary) (see docs on API documentation).
  • Quality standards are met.

@themarolt themarolt added the Bug Created by Linear-GitHub Sync label Oct 13, 2023
@themarolt themarolt requested a review from epipav October 13, 2023 09:57
@themarolt themarolt merged commit b91a56c into main Oct 13, 2023
8 checks passed
@themarolt themarolt deleted the bugfix/transaction-handling-for-member-and-org-merge-C-2425 branch October 13, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Created by Linear-GitHub Sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants