-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: Remove related activity #1278
Conversation
📝 WalkthroughWalkthroughThis pull request primarily focuses on the removal of the Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
Quality Gate passedIssues Measures |
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (2)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Activities/DialogActivity.cs (2)
Line range hint
1-37
: Approved: Removal of related activity propertiesThe removal of
RelatedActivityId
,RelatedActivity
, andRelatedActivities
properties aligns with the PR objective of removing the related activity feature. This simplification of the class structure is appropriate given that the feature is not currently in use.Consider adding a comment explaining why these properties were removed, to provide context for future developers:
// Related activity properties removed as part of feature cleanup (PR #1278)
Also applies to: 39-57
Line range hint
1-57
: Summary: Successful removal of related activity featureThe changes in this file effectively remove the related activity feature from the
DialogActivity
class. This includes:
- Removal of
RelatedActivityId
,RelatedActivity
, andRelatedActivities
properties.- Updating the
OnCreate
method to remove theRelatedActivityId
parameter from the domain event creation.These changes align well with the PR objectives and simplify the class structure. The removal of unused features helps in maintaining a cleaner codebase.
As the codebase evolves, consider periodically reviewing and removing unused features to prevent accumulation of unnecessary code. This practice helps in maintaining a lean and efficient codebase.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (24)
- docs/schema/V1/schema.verified.graphql (0 hunks)
- docs/schema/V1/swagger.verified.json (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogActivityEventToAltinnForwarder.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Get/GetDialogActivityDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Search/SearchDialogActivityDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/SearchDialogDtoBase.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/GetDialogActivityDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Search/SearchDialogActivityDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommand.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogDto.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/SearchDialogDtoBase.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Activities/DialogActivity.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/Activities/DialogActivityCreatedDomainEvent.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/ObjectTypes.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Configurations/Dialogs/Activities/DialogActivityConfiguration.cs (0 hunks)
- src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20241011235344_RemoveRelatedActivity.Designer.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20241011235344_RemoveRelatedActivity.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/DialogDbContextModelSnapshot.cs (1 hunks)
- tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs (0 hunks)
💤 Files with no reviewable changes (19)
- docs/schema/V1/schema.verified.graphql
- docs/schema/V1/swagger.verified.json
- src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/DialogActivityEventToAltinnForwarder.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Get/GetDialogActivityDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogActivities/Queries/Search/SearchDialogActivityDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Search/SearchDialogDtoBase.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/GetDialogActivityDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Search/SearchDialogActivityDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogCommandValidator.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommand.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogCommandValidator.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogDto.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Search/SearchDialogDtoBase.cs
- src/Digdir.Domain.Dialogporten.GraphQL/EndUser/Common/ObjectTypes.cs
- src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Configurations/Dialogs/Activities/DialogActivityConfiguration.cs
- tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Dialogs/Commands/UpdateDialogTests.cs
🧰 Additional context used
📓 Learnings (3)
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20241011235344_RemoveRelatedActivity.Designer.cs (4)
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-09-16T06:47:29.219Z Learning: Do not consider style or maintainability issues in migration files generated by Entity Framework during code reviews.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-10-08T15:39:24.789Z Learning: Do not consider style or maintainability issues in migration files generated by Entity Framework during code reviews.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-10-08T15:39:24.789Z Learning: Ensure that changes to domain entities are accompanied by corresponding migrations.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-09-16T06:47:29.219Z Learning: Ensure that changes to domain entities are accompanied by corresponding migrations.
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20241011235344_RemoveRelatedActivity.cs (4)
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-09-16T06:47:29.219Z Learning: Do not consider style or maintainability issues in migration files generated by Entity Framework during code reviews.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-10-08T15:39:24.789Z Learning: Do not consider style or maintainability issues in migration files generated by Entity Framework during code reviews.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-10-08T15:39:24.789Z Learning: Ensure that changes to domain entities are accompanied by corresponding migrations.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-09-16T06:47:29.219Z Learning: Ensure that changes to domain entities are accompanied by corresponding migrations.
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/DialogDbContextModelSnapshot.cs (2)
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-10-08T15:39:24.789Z Learning: Do not consider style or maintainability issues in migration files generated by Entity Framework during code reviews.
Learnt from: elsand PR: digdir/dialogporten#1110 File: src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20240910063003_AddDialogActivityDialogOpened.cs:13-16 Timestamp: 2024-09-16T06:47:29.219Z Learning: Do not consider style or maintainability issues in migration files generated by Entity Framework during code reviews.
🔇 Additional comments (8)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/Activities/DialogActivityCreatedDomainEvent.cs (1)
Line range hint
1-14
: LGTM! The change aligns with the PR objectives.The removal of the
RelatedActivityId
parameter from theDialogActivityCreatedDomainEvent
record is consistent with the goal of removing the related activity feature. This change simplifies the data model and removes unnecessary relationships.To ensure consistency across the codebase, please run the following script to check for any remaining references to
RelatedActivityId
in the context of dialog activities:This script will help identify any places where
RelatedActivityId
might still be referenced, ensuring that the removal is complete and consistent throughout the codebase.src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20241011235344_RemoveRelatedActivity.cs (3)
9-10
: LGTM: Migration class declaration is appropriate.The migration class name
RemoveRelatedActivity
accurately reflects the purpose of this migration, which aligns with the PR objectives to remove the related activity feature.
12-25
: LGTM: Up method correctly implements the removal of RelatedActivityId.The
Up
method properly removes theRelatedActivityId
column and its associated constraints from theDialogActivity
table. The sequence of operations is correct:
- Drops the foreign key constraint.
- Removes the index.
- Deletes the column.
This implementation aligns with the PR objectives to remove the related activity feature.
28-48
: LGTM: Down method correctly reverses the migration.The
Down
method properly reinstates theRelatedActivityId
column and its associated constraints in theDialogActivity
table. The sequence of operations is correct:
- Adds the column back.
- Creates the index.
- Adds the foreign key constraint.
This implementation provides a proper rollback mechanism if needed.
Note: The foreign key constraint is set with
onDelete: ReferentialAction.SetNull
. This means that if a related activity is deleted, theRelatedActivityId
will be set to null rather than causing a cascading delete or throwing an error. This behavior aligns with the original schema design.src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Activities/DialogActivity.cs (1)
38-38
: Approved: Updated domain event creationThe removal of the
RelatedActivityId
parameter from theDialogActivityCreatedDomainEvent
constructor is consistent with the removal of related activity properties.To ensure this change doesn't break any existing code, please verify that all usages of
DialogActivityCreatedDomainEvent
have been updated accordingly. Run the following script to check for any remaining references:✅ Verification successful
Verified: Removal of
RelatedActivityId
ParameterAll constructor calls to
DialogActivityCreatedDomainEvent
have been updated to remove theRelatedActivityId
parameter. No remaining references were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to RelatedActivityId in DialogActivityCreatedDomainEvent usage # Search for DialogActivityCreatedDomainEvent constructor calls rg --type csharp -A 10 "new DialogActivityCreatedDomainEvent\("Length of output: 1399
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/DialogDbContextModelSnapshot.cs (2)
20-20
: Entity Framework Core version update approved.The
ProductVersion
has been updated from "8.0.8" to "8.0.10", which likely reflects an upgrade in the Entity Framework Core version used in the project. This update is appropriate and aligns with keeping the project dependencies up-to-date.
Line range hint
1-1023
: Removal of RelatedActivityId from DialogActivity entity.The
RelatedActivityId
property has been removed from theDialogActivity
entity, which is consistent with the PR objective of removing the related activity feature. This change appears to be intentional and aligns with the desired modifications outlined in the pull request.src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20241011235344_RemoveRelatedActivity.Designer.cs (1)
1-1984
: Migration file is properly generatedThe migration file
RemoveRelatedActivity
appears to be correctly generated by Entity Framework to remove therelatedActivityId
field as per the PR objectives. No issues detected.
Description
Marking this as a
chore
. "Silently" removing this feat, there are no dialogs in staging that uses this.Removes related activity on DialogActivity.
Related Issue(s)
Verification
Documentation
docs
-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
relatedActivityId
field from various models, enhancing clarity in dialog and activity management.Bug Fixes
relatedActivityId
, streamlining the validation process for dialog activities.Documentation
relatedActivityId
across multiple data structures, improving documentation clarity.Tests