-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Required fields and default values #5835
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5835 +/- ##
==========================================
- Coverage 93.75% 93.14% -0.61%
==========================================
Files 148 150 +2
Lines 10304 10616 +312
==========================================
+ Hits 9660 9888 +228
- Misses 644 728 +84
Continue to review full report at Codecov.
|
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.
Thanks for getting started on this! I have a quick comment.
type, | ||
targetClass, | ||
}); | ||
if (fieldOptions && Object.keys(fieldOptions).length > 0) { |
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.
Can you add a test for setting required fields and defaults when you create a new class / new class schema?
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.
Are the default values validated? Can I set a number to a type String |
@dplewis It is a good question. They aren't validated. I will work on this. |
The StorageAdapters should handle the validation for you no? Can you write a test for this? |
No. I've written a test and it is not validating. I'm fixing it and should push new commits by the end of the day. |
@dplewis it is now validating. Can you please take a look again? Note that I also fixed a bug. Currently, when the addClassIfNotExists is called, any validation error (for example, invalid class name) are returned in a wrong format (not a Parse.Error). As a result, the API is returning |
How would this work with beforeSave trigger? I think you should be good as it looks like changes made in a beforeSave would override default values. |
I think that the beforeSave trigger run before the validations and default assignments. So, if you set something in beforeSave, it will stay. On the other hand, if you set a required field to null in the beforeSave trigger, it will set the default value or return an error in the case it is a required field. This is the behavior I planned and I think it's consistent. Do you agree? I will add some tests to check. |
…use of unset in the beforeSave trigger
@dplewis Added some tests for beforeSave trigger and fixed a bug. The default value was not applied if you unset in beforeSave. |
That will help me a lot. Thanks @davimacedo. A little suggestion for later: the strings fields should have a uppercase or lowercase option to automatically converts it. What do you think? |
@joaovq there is a lot of things that we can still do here, as regular expression validation, formatted output (I think the lowercase/uppercase scenario goes here), etc. I will probably try to address some of these features in the future. But each of these features requires a lot of work not only in the Parse Server side, but also in docs, Parse Dashboard, and SDK. So, any help here would be very appreciated. Would you be able to tackle some of these tasks? |
I agree with you. It's really a lot to do and time flies. I had to a lot of
things on my own to overcome some obstacles that I had so I think I would
be able to tackle some of these if a short deadline is not required.
Em sex, 26 de jul de 2019 às 13:23, Antonio Davi Macedo Coelho de Castro <
[email protected]> escreveu:
… @joaovq <https://github.com/joaovq> there is a lot of things that we can
still do here, as regular expression validation, formatted output (I think
the lowercase/uppercase scenario goes here), etc. I will probably try to
address some of these features in the future. But each of these features
requires a lot of work not only in the Parse Server side, but also in docs,
Parse Dashboard, and SDK. So, any help here would be very appreciated.
Would you be able to tackle some of these tasks?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5835?email_source=notifications&email_token=ABJGV4TJ4VKNLVA3I2NF5OTQBMQGNA5CNFSM4IFVYO7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25CJ7Y#issuecomment-515515647>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABJGV4V47VQZVKKZ3TLA263QBMQGNANCNFSM4IFVYO7A>
.
|
That's nice @joaovq . There is no short deadline. Any effort that you can put to help on this is very welcome! |
Well then I'm in. What do I have to do to start?
I've never did this before.
Em sex, 26 de jul de 2019 às 19:19, Antonio Davi Macedo Coelho de Castro <
[email protected]> escreveu:
… That's nice @joaovq <https://github.com/joaovq> . There is no short
deadline. Any effort that you can put to help on this is very welcome!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5835?email_source=notifications&email_token=ABJGV4QL4Y6FF2TARXJ7MODQBNZ7VA5CNFSM4IFVYO7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2522YQ#issuecomment-515616098>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABJGV4WHY5NRWFE6Q2NA53LQBNZ7VANCNFSM4IFVYO7A>
.
|
Here it goes few things that I see as next steps:
Then we should do similar steps for each new option we want to include in a schema field. What do you think? If you agree with this plan I can open a project with these tasks so we can keep track of what we have already done. |
Nice. I agree with it. You can open the project
Do you wanna know what I want to do? Or can I do it and then tell to you?
Em seg, 29 de jul de 2019 às 02:05, Antonio Davi Macedo Coelho de Castro <
[email protected]> escreveu:
… @joaovq <https://github.com/joaovq>
Here it goes few things that I see as next steps:
- Add the required and defaultValue options in the dashboard when
creating a new column - I think @alencarlucas
<https://github.com/alencarlucas> is already working on this
- Improve GraphQL server to use required and defaultValue options when
generating the schema - I will probably tackle this soon
- Modify the docs explaining about these new options
- Include in Parse Server the possibility of changing these settings
(currently it is not possible to modify a schema field)
- Include in Parse Dashboard the option to modify a schema field
Then we should do similar steps for each new option we want to include in
a schema field.
What do you think? If you agree with this plan I can open a project with
these tasks so we can keep track of what we have already done.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5835?email_source=notifications&email_token=ABJGV4WPWTPDFW47CQEVWTLQBZ3ANA5CNFSM4IFVYO7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD27S5HQ#issuecomment-515845790>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABJGV4TQPSYVFUSURHYQUX3QBZ3ANANCNFSM4IFVYO7A>
.
|
Here is the project: https://github.com/parse-community/parse-server/projects/7 Feel free to just do whatever task you want and send the PR, then some of the maintainers will review. But, you might want to move the task you are working on, so it will avoid other developer working in the same issue. Thanks for your help! |
Nice. I will fork the project and take a look at it. I guess I should see
the changes you made to figure out that part of the code, otherwise I'll be
completely lost. What do you think?
As soon as I get the idea I'll start.
Em ter, 30 de jul de 2019 às 18:46, Antonio Davi Macedo Coelho de Castro <
[email protected]> escreveu:
… Here is the project:
https://github.com/parse-community/parse-server/projects/7
Feel free to just do whatever task you want and send the PR, then some of
the maintainers will review. But, you might want to move the task you are
working on, so it will avoid other developer working in the same issue.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5835?email_source=notifications&email_token=ABJGV4SOGLFDH6U5UYNXXSTQCCZCVA5CNFSM4IFVYO7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3FNBZA#issuecomment-516608228>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABJGV4V5FHAVO36MHBP3DDDQCCZCVANCNFSM4IFVYO7A>
.
|
Sure. That's a good way to get started. Also let me know if you have any question. |
Ok, I probably will haha. Thank you
Em ter, 30 de jul de 2019 às 21:38, Antonio Davi Macedo Coelho de Castro <
[email protected]> escreveu:
… Sure. That's a good way to get started. Also let me know if you have any
question.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5835?email_source=notifications&email_token=ABJGV4VILKXJDDZPOOTGYPDQCDNJVA5CNFSM4IFVYO7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3FWUQA#issuecomment-516647488>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABJGV4WGFUF64JZPUUZ4V2TQCDNJVANCNFSM4IFVYO7A>
.
|
Just wanted to ask if this change will be included in the parse-dashboard? |
@JeromeDeLeon It's already included, take a look in this PR. |
How about parse-js-sdk? |
What do you have in mind for the JS SDK? |
Nah, nvm. I just thought of it but then I realized that using parse-dashboard is fine. |
Is this feature documented yet? |
Closes: #930 Requires Parse Server 3.7.0+ Feature: parse-community/parse-server#5835 I added a FieldOption to addField and its counter parts. This will allow for future options like uppercase / lowercase for example. I keep getting a schema mismatch when I use dates and pointers. @davimacedo Maybe you know why?
* Parse.Schema required fields and defaultValues Closes: #930 Requires Parse Server 3.7.0+ Feature: parse-community/parse-server#5835 I added a FieldOption to addField and its counter parts. This will allow for future options like uppercase / lowercase for example. I keep getting a schema mismatch when I use dates and pointers. @davimacedo Maybe you know why? * improve coverage * fix date and pointer fields * print tests * fix utc date test * Documentation * nit test
* Add field options to mongo schema metadata * Add/fix test with fields options * Add required validation failing test * Add more tests * Only set default value if field is undefined * Fix redis test * Fix tests * Test for creating a new class with field options * Validate default value type * fix lint (weird) * Fix lint another way * Add tests for beforeSave trigger and solve small issue regarding the use of unset in the beforeSave trigger
and who can I remove the required flag from the field without data loss? |
@rasool1994 You can remove it from the database without any data loss. Just connect to your mongoDB instance and go to SCHEMA collection see example below: "_id" : "PostCategory",
"objectId" : "string",
"updatedAt" : "date",
"createdAt" : "date",
"_metadata" : {
"fields_options" : {
"arName" : {
"required" : true
},
"enName" : {
"required" : true
},
"channelName" : {
"required" : false
}
}, |
This PR adds the capability for defining required fields and default values to the schema. For example:
How it works:
close #3587