-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update predefined pages #2550
base: main
Are you sure you want to change the base?
Update predefined pages #2550
Conversation
Return value isn't nullable due to using `findOneOrFail`.
Make sure that saving a predefined page document actually works.
We do this for the Admin Generator, so we should do it here as well.
Predefined pages aren't structured content.
62bf9ee
to
5d7b864
Compare
I tried splitting the save mutation into a create and update mutation first, but this requires a change to DocumentInterface, so I reverted my change. Should we plan this for the future? |
@Field(() => String, { nullable: true }) | ||
@IsString() | ||
@Field(() => PredefinedPageType, { nullable: true }) | ||
@IsEnum(PredefinedPageType) | ||
@IsOptional() |
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.
@nsams type can be both optional (not changed) and nullable (unset). Should I use @IsUndefinable
and @IsNullable()
instead? And should the TS type reflect that as well?
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.
yes, @IsOptional
should not be used anymore. And in that case @IsUndefinable
and @IsNullable()
together are correct.
And should the TS type reflect that as well?
yes
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.
Fixed: be85243
@nsams @thomasdax98 it's probably best to review this on a commit basis. |
Description
The predefined pages demo was in a bad shape: Loading/saving didn't work, the info tag didn't look right, etc. I've therefore updated the demo to reflect the latests changes/decisions made in the CRUD Generator. Notable changes:
type
Related tasks and documents
https://vivid-planet.atlassian.net/browse/COM-1080
Further information
The implementation in Demo will be the basis for the docs, so please review carefully.