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

#604 Add existence and type check for value passed to computed prop #652

Merged
merged 5 commits into from
Jul 13, 2020

Conversation

crablab
Copy link
Contributor

@crablab crablab commented Jul 8, 2020

Adds existence and type checking for the val parameter passed to the computed prop, to handle the case that a null value is passed.

@crablab crablab requested a review from YaaL July 8, 2020 09:47
@crablab crablab requested a review from warrensearle July 9, 2020 14:13
@crablab crablab requested a review from warrensearle July 13, 2020 10:14
Copy link
Member

@warrensearle warrensearle left a comment

Choose a reason for hiding this comment

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

@crablab can you double check the required behaviour here.
This change means that user will be unable to empty a value from the 'estimated launch date' field.
I would expect emptying the month and year to be a save-able change

@crablab
Copy link
Contributor Author

crablab commented Jul 13, 2020

@warrensearle Ah, it was my understanding that this should not be possible.

To confirm, we want to allow a null value to be saved? In which case, we just need to handle that case rather than trying to transform a null value into a date 👍

Copy link
Member

@warrensearle warrensearle left a comment

Choose a reason for hiding this comment

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

Providing we're happy that once a user has provided a launch date they can't remove it then this is good to go

@crablab
Copy link
Contributor Author

crablab commented Jul 13, 2020

@ClaireTroughtonJAC Would you be able to just confirm the understanding here either way?

Understanding: Once a launch date has been provided (eg. when the Exercise is created) it can be modified, but it cannot be set to null (ie. no launch date).

@crablab
Copy link
Contributor Author

crablab commented Jul 13, 2020

Claire has confirmed this is correct 👍

@crablab crablab merged commit eda175b into develop Jul 13, 2020
@crablab crablab deleted the feature/604-summary-missing-getUTCFullYear branch July 13, 2020 16:23
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