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

feat: add section models to corpus db #252

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

jpetto
Copy link
Contributor

@jpetto jpetto commented Jan 6, 2025

Goal

add section models to corpus db to enable ML to create sections for new tab.

  • rename ScheduledItemSource enum to more broad ActivitySource

I'd love feedback/perspectives on:

  • the active and deactivateSource fields - do those make sense?

Implementation Decisions

  • eventually, these table will need to support editorially created sections. they will almost surely be altered once those requirements are set, but here trying my best to keep the models flexible enough for future expansion/alteration.

Deployment steps

  • Deployed to dev?

References

JIRA ticket:

@jpetto jpetto force-pushed the MC-1614-add-sections-tables-to-corpus branch 3 times, most recently from 6ffc7e5 to 9fa2aee Compare January 6, 2025 17:41
// we will not delete Sections, instead deactivate them
active Boolean @default(false)
// track who deactivated the Section
deactivateSource ActivitySource // (ML or MANUAL)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add the following:

updatedBy (or updatedSource?)
deactivatedAt

active Boolean @default(true)
// track who deactivated the SectionItem
deactivateSource ActivitySource // (ML or MANUAL)
createdAt DateTime @default(now())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will add deactivedAt

- rename `ScheduledItemSource` enum to more broad `ActivitySource`
@jpetto jpetto force-pushed the MC-1614-add-sections-tables-to-corpus branch from 9fa2aee to 09380a4 Compare January 6, 2025 20:50
@jpetto jpetto marked this pull request as ready for review January 6, 2025 20:55
`sort` INTEGER NOT NULL,
`active` BOOLEAN NOT NULL DEFAULT false,
`createSource` ENUM('MANUAL', 'ML') NOT NULL,
`deactivateSource` ENUM('MANUAL', 'ML') NOT NULL,
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙌

}

model SectionItem {
id Int @id @default(autoincrement())
Copy link
Contributor

@rolf-moz rolf-moz Jan 6, 2025

Choose a reason for hiding this comment

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

Wondering if plain id is required here and whether a single externalId would suffice as the id. Or is this the design pattern with primsa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's true that we don't need the integer id at the moment. this is more keeping in convention with the rest of the db design - using auto-incrementing ids for internal ids/associations. it's a small data point and shouldn't impact performance.

rolf-moz
rolf-moz previously approved these changes Jan 6, 2025
@jpetto jpetto dismissed stale reviews from rolf-moz and katerinachinnappan via e201def January 7, 2025 16:09
@github-actions github-actions bot deployed to prospect-api-dev January 7, 2025 16:25 Active
@github-actions github-actions bot deployed to curated-corpus-api-dev January 7, 2025 16:26 Active
@github-actions github-actions bot deployed to collection-api-dev January 7, 2025 16:29 Active
@jpetto jpetto merged commit c34cd7b into main Jan 7, 2025
54 checks passed
@jpetto jpetto deleted the MC-1614-add-sections-tables-to-corpus branch January 7, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants