custom_fields: added custom vocab flag #1109
Conversation
0einstein0
force-pushed
the
Custom-Fields
branch
2 times, most recently
from
652ad0a
to
7b7d3cb
Compare
...mmunities/assets/semantic-ui/js/invenio_communities/settings/profile/CommunityProfileForm.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Couple of questions/comments, but otherwise looks good (with @yashlamba)
...mmunities/assets/semantic-ui/js/invenio_communities/settings/profile/CommunityProfileForm.js
Outdated
Show resolved
Hide resolved
| @@ -28,16 +28,25 @@ export class CustomFieldSerializer { | |||
| if (customFields !== null) { | |||
| for (const [key, value] of Object.entries(customFields)) { | |||
| const isVocabularyField = this.vocabularyFields.includes(key); | |||
| const isCustomVocabulary = value.isCustomVocabulary; | |||
There was a problem hiding this comment.
comment/minor: why do we have to track separately if the value is coming from a custom vocabulary? also, what is a "custom vocabulary" compared to a regular vocabulary? Aren't they served under /api/vocabularies/....
If it's about being "generic" vocabularies, I think we should use the same terminology, i.e. isGenericVocabulary.
0einstein0
force-pushed
the
Custom-Fields
branch
3 times, most recently
from
06fb5c1
to
4062259
Compare
| const _value = _isArray(value) | ||
| ? value.map((v, i) => mapValue(v, i, isVocabularyField)) | ||
| : mapValue(value, null, isVocabularyField); | ||
| ? value.map((v, i) => mapValue(v, i, isVocabularyField, isGenericVocabulary)) |
There was a problem hiding this comment.
I think now the isVocabularyField is quite confusing as name because isGenericVocabulary should also be a vocabulary field. Do you intend to split between generic and custom?
There was a problem hiding this comment.
Yes because the custom field we added is a non generic vocabulary, hence why this check was needed.
There was a problem hiding this comment.
then I would clearly rename isVocabularyField with something like isCustomVocabularyField
0einstein0
force-pushed
the
Custom-Fields
branch
from
4062259
to
f344e8b
Compare
0einstein0
force-pushed
the
Custom-Fields
branch
from
f344e8b
to
2d20371
Compare
❤️ Thank you for your contribution!
Description
Closes Issue #733
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Third-party code
If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: