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

chore: use db model instead of content type #19563

Merged
merged 6 commits into from Feb 21, 2024
Merged

chore: use db model instead of content type #19563

merged 6 commits into from Feb 21, 2024

Conversation

remidej
Copy link
Contributor

@remidej remidej commented Feb 20, 2024

What does it do?

Removes the history version content type. Registers a history version model instead.

Why is it needed?

We want to stop using content types for internal stuff, as it doesn't need the CM, CTB, review workflows etc.

How to test it?

Everything should work as it did before

@remidej remidej added the source: core:content-manager Source is core/content-manager package label Feb 20, 2024
@remidej remidej added the pr: chore This PR contains chore tasks (cleanups, configs, tooling...) label Feb 20, 2024
@remidej remidej self-assigned this Feb 20, 2024

This comment was marked as spam.

Copy link
Contributor

@markkaylor markkaylor left a comment

Choose a reason for hiding this comment

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

Cool! I think this causes problems for single-types though. Strangely it is still able to fetch them but I get errors when selecting different versions:

Screenshot 2024-02-21 at 08 46 23

I think it is related to this line:

const isSingleType = strapi.getModel(contentTypeUid).kind === 'singleType';

Not sure the API works the same for models.

Comment on lines +39 to +44
createdBy: {
type: 'relation',
relation: 'oneToOne',
target: 'admin::user',
useJoinTable: false,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

is it necessary to use a join table for the created by field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it would be? Here it's false, and for normal content types it's false as well

Copy link
Contributor

Choose a reason for hiding this comment

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

didn't select the right section 😓 I think the attribute is correctly set up and will not use a join table, but there is a message about jointable being optional and didn't get why 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see. The comment is because TS wanted us to provide a joinTable property (not sure about the name exactly). But it shouldn't be required when useJoinTable is false. The type isn't smart enough for that yet, that's why we expect a TS error here

@remidej remidej merged commit ab39246 into v5/history Feb 21, 2024
7 of 8 checks passed
@remidej remidej deleted the history/model branch February 21, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: chore This PR contains chore tasks (cleanups, configs, tooling...) source: core:content-manager Source is core/content-manager package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants