-
Notifications
You must be signed in to change notification settings - Fork 5
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
EES-4490: Add theme to all publications #5269
base: dev
Are you sure you want to change the base?
Conversation
5a72797
to
1aee989
Compare
name: "FK_Publications_Topics_TopicId", | ||
table: "Publications"); | ||
|
||
migrationBuilder.Sql(@" |
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.
After this point, could we then add a foreign key constraint to this column, once all of the correct values are populated?
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.
A foreign key was added automatically through changes to the class (see the snapshot file to see if that's what you meant).
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.
But I still think that you need to add the SQL to enforce a foreign key. The snapshot update doesn't create SQL commands itself - it's just a mechanism to let the next migration know what has changed since the last migration was created, as far as I know. Would you mind checking in your DB to see if a foreign key constraint is created?
src/explore-education-statistics-admin/src/components/form/FormFieldThemeTopicSelect.tsx
Show resolved
Hide resolved
src/GovUk.Education.ExploreEducationStatistics.Admin/Services/PublicationService.cs
Show resolved
Hide resolved
src/GovUk.Education.ExploreEducationStatistics.Admin/Services/PublicationService.cs
Show resolved
Hide resolved
...tatistics.Admin/Migrations/ContentMigrations/20240916141644_EES4490_AddThemeToPublication.cs
Outdated
Show resolved
Hide resolved
826d2f7
to
95b809f
Compare
95b809f
to
1bdd037
Compare
This PR adds a
Theme
property directly to aPublication
, in preparation for the removal ofTopic
.The work includes:
Publication
column and populate existing records with the correct theme ID, andIt was fairly awkward deciding which aspects could/should be changed in advance of the removal of topics, and which should remain, as the affected entities are inherently linked, and removing or changing them could break existing functionality until the next piece of work to remove topics is completed.