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

Fix CDS prop nullability if @mandatory is used #315

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

siarhei-murkou
Copy link
Contributor

Hi @daogrady 👋 ,

Since the version 8.0.2, CDS has introduced the new logic for CDS params in CDS functions/actions. Now, we have to use @mandatory annotation instead of setting not null keyword. Whereas not null is still working on CDS entity layer..
Therefore, I have added a check on @mandatory existence in order to determine if the property should/shouldn't be marked as nullable in the compiled TypeScript files.

@daogrady
Copy link
Contributor

Hi Siarhei,

thanks for providing this addition! Looks good on first glance, but there seem to be no tests covering this new behaviour as of now. Can you please add a test checking a mandatory vs a non-mandatory field? A more approachable way of testing has been added recently, where you basically have to write consuming TS code for your generated files. Let me know when you need help writing this test.

Best,
Daniel

@daogrady
Copy link
Contributor

Also, as I have learned today, this feature may still slightly change in the near future. So let's maybe shelf this for now until we know with certainty how @mandatory and not null will turn out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants