Initial PoC of seeder API #6424
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## arch/seeder-sdk #6424 +/- ##
===================================================
- Coverage 50.73% 50.71% -0.02%
===================================================
Files 1869 1867 -2
Lines 82798 82701 -97
Branches 7322 7307 -15
===================================================
- Hits 42004 41945 -59
+ Misses 39191 39158 -33
+ Partials 1603 1598 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
New Issues (1)Checkmarx found the following issues in this Pull Request
Fixed Issues (1)Great job! The following issues were fixed in this Pull Request
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| { | ||
| public Guid Id { get; set; } | ||
| public required string RecipeName { get; set; } | ||
| public required string Data { get; set; } // JSON blob with entity tracking info |
There was a problem hiding this comment.
❌ We should refrain from inline comments as they don't help us craft clean code. I recommend that we either remove or add an XML comment but I prefer the prior and avoid it altogether.
There was a problem hiding this comment.
❌ None of the comments in the Program.cs file help the next teammate understand the program better than intellisense already does. I recommend that we remove them.
MGibson1
force-pushed
the
arch/seeder-api
branch
from
abd2d4b to
2bbf7d1
Compare
MGibson1
force-pushed
the
arch/seeder-api
branch
from
2bbf7d1 to
d93cd50
Compare
Fixup single user recipe to inform of seeded entities
…ch/seeder-api # Conflicts: # util/Seeder/Factories/UserSeeder.cs
…ch/seeder-api # Conflicts: # util/Seeder/Factories/UserSeeder.cs
| [Active] BIT NOT NULL CONSTRAINT [DF_Device_Active] DEFAULT (1), | ||
| CONSTRAINT [PK_Device] PRIMARY KEY CLUSTERED ([Id] ASC), | ||
| CONSTRAINT [FK_Device_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) | ||
| CONSTRAINT [FK_Device_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) ON DELETE CASCADE |
There was a problem hiding this comment.
@MGibson1 did we want to revert this?
There was a problem hiding this comment.
That was undecided. I believe we don't need it anymore with the full repository injection we're using, but it's not wrong.
@rkac-bw @withinfocus Care to comment on using cascading delete here vs needing to understand delete through sprocs?
There was a problem hiding this comment.
I am not a fan of cascades personally and try to avoid them, but they were here when I showed up so I am working with it. Our approach -- and this could use documentation on our revamped SQL code style page -- is to manage deletions that touch critical tables explicitly e.g. Organization_DeleteById, and for smaller children of those tables to allow cascades so as to keep the coding pattern simple enough.
One example: OrganizationIntegration, being linked to the critical Organization table, is deleted via the above proc. Its child table OrganizationIntegrationConfiguration has a cascade delete on it.
|
4 participants
🎟️ Tracking
📔 Objective
Adds a standalone API service which allows you to run and destroy seeds. The intention is to allow integration tests to seed the database.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes