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

<DO NOT MERGE> Coding dojo: refactor to apply aggregate root pattern in DiseaseOutbreakEvent + IncidentManagementTeam #29

Open
wants to merge 15 commits into
base: development
Choose a base branch
from

Conversation

anagperal
Copy link
Contributor

@anagperal anagperal commented Oct 18, 2024

📌 References

📝 Implementation

  • Delete IncidentManagementTeamD2Repository and add functions directly to DiseaseOutbreakEventD2Repository
  • Delete from CompositionRoot functions related to incidentManagementTeam and move it to diseaseOutbreakEvent
  • Delete getIncidentManagementTeamMember (in repo and usecase) and get in GetDiseaseOutbreakByIdUseCase the incident manager using the incidentManagerName filtering incidentManagementTeam
  • Review deleteIncidentManagementTeamMemberRole and saveIncidentManagementTeamMemberRole and have in params only necessary, do not condition the domain to what the repo endpoint needs (remove roles from params and improve code)
  • create GetDiseaseOutbreakEventAggregateRootByIdUseCase, DiseaseOutbreakEventAggregateRoot and IncidentManagementTeamInAggregateRoot in order to apply aggregate root pattern in Incident Management Team Builder page
  • Create the Incident Management Team in hook useIncidentManagementTeamView from IncidentManagementTeamInAggregateRoot + TeamMember[] + Role[] = new entity in presentation layer
  • Add roles and team members in AppContext
  • Delete all related to IncidentManagementTeam as entity and leave it as IncidentManagementTeamInAggregateRoot. IncidentManagementTeam will be the entity used in Presentation Layer
  • Add comments in functions and classes with aggregate root working in parallel with old code (add @ deprecated)

Future:

  • Create/update/delete of children done through DiseaseOutbreakEventAggregateRoot entity
  • In DiseaseOutbreakEventRepository when save IncidentManagementTeamMemberRole send DiseaseOutbreakEventAggregateRoot and check what is new to save it

📹 Screenshots/Screen capture

🔥 Notes to the tester

@anagperal anagperal changed the base branch from development to fix/incident-management-team-fixes November 7, 2024 18:07
@anagperal anagperal requested a review from xurxodev November 8, 2024 07:32
@anagperal anagperal self-assigned this Nov 8, 2024
Base automatically changed from fix/incident-management-team-fixes to development November 11, 2024 10:52
@anagperal anagperal marked this pull request as ready for review November 14, 2024 15:05
…ot to addTeamMemberToIncidentManagementTeamHierarchy
Copy link

@xurxodev xurxodev left a comment

Choose a reason for hiding this comment

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

thanks @anagperal

I approve this PR because it is the beginning of a very big refactor and some things are temporal.

However, I leave here all things I saw:

  • In the future, DiseaseOutbreakEventTestRepository only should have: get, getById, save and delete. We should save and return always the aggregate root, and we should not have methods to retrieve only children.
  • Great to request roles and team members to start the app using app context but we should not have GetConfigurationsUseCase and GetConfigurationsRepository. roles, teamMembers and incidentManagers are different entities y should have different use cases and repositories, all called to start the app. Configurations entity doesn’t exist.
  • I understand that the keyword AgreggateRoot is to identify old code and new code, in the future I would remove this keyword of entities, functions, etc..
  • Great to add validations to new aggregate root to addTeamMemberToIncidentManagementTeamHierarchy, in the future we would need add tests for this validations using the aggregate root
  • In the future all validations related to aggregate root should be done in the entity

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

Successfully merging this pull request may close these issues.

2 participants