Skip to content

Commit

Permalink
handle validate for choices (#4793)
Browse files Browse the repository at this point in the history 
* handle validate for choices

* reset enableValidate on getChoices

* add support for nested parent visibility

* rename variable

* considers field not visible if parent is not neither

* validate without emit to prevent unsaved changes modal

* update changelog

* fix lint issue

---------

Co-authored-by: Harouna Traoré <[email protected]>
Co-authored-by: Jed <[email protected]>
  • Loading branch information
@haroun @ValJed
3 people authored Nov 13, 2024
1 parent 99b71ba commit 7a7f8a4
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 20 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ did not actually use any noncompliant cookie names or values, so there was no vu
### Changes

* Removes postcss plugin and webpack loader used for breakpoint preview mode. Uses instead the new `postcss-viewport-to-container-toggle` plugin in the webpack config.
* Removes error messages in server console for hidden fields. These messages should not have been printed out in the server console in the first place.
* Removes invalid error messages on select fields appearing while opening an existing valid document.

## 4.9.0 (2024-10-31)

Expand Down
46 changes: 35 additions & 11 deletions modules/@apostrophecms/schema/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,10 +577,21 @@ module.exports = {
// `destination` (the current level). This allows resolution of relative
// `following` paths during sanitization.

async convert(req, schema, data, destination, { fetchRelationships = true, ancestors = [] } = {}) {
async convert(
req,
schema,
data,
destination,
{
fetchRelationships = true,
ancestors = [],
isParentVisible = true
} = {}
) {
const options = {
fetchRelationships,
ancestors
ancestors,
isParentVisible
};
if (Array.isArray(req)) {
throw new Error('convert invoked without a req, do you have one in your context?');
Expand All @@ -603,6 +614,9 @@ module.exports = {

if (convert) {
try {
const isAllParentsVisible = isParentVisible === false
? false
: await self.isVisible(req, schema, destination, field.name);
const isRequired = await self.isFieldRequired(req, field, destination);
await convert(
req,
Expand All @@ -612,7 +626,10 @@ module.exports = {
},
data,
destination,
options
{
...options,
isParentVisible: isAllParentsVisible
}
);
} catch (error) {
if (Array.isArray(error)) {
Expand All @@ -635,7 +652,9 @@ module.exports = {
if (error.path) {
// `self.isVisible` will only throw for required fields that have
// an external condition containing an unknown module or method:
const isVisible = await self.isVisible(req, schema, destination, error.path);
const isVisible = isParentVisible === false
? false
: await self.isVisible(req, schema, destination, error.path);

if (!isVisible) {
// It is not reasonable to enforce required,
Expand All @@ -652,10 +671,15 @@ module.exports = {
// for a field that is not visible should be quietly discarded.
// We only worry about this if the value is not valid, as otherwise
// it's a kindness to save the work so the user can toggle back to it
destination[field.name] = klona((field.def !== undefined) ? field.def : self.fieldTypes[field.type]?.def);
destination[field.name] = klona((field.def !== undefined)
? field.def
: self.fieldTypes[field.type]?.def);
continue;
}
}
if (isParentVisible === false) {
continue;
}
}

if (!Array.isArray(error) && typeof error !== 'string') {
Expand Down Expand Up @@ -907,7 +931,7 @@ module.exports = {
const find = manager.find;

const options = {
find: find,
find,
builders: { relationships: withRelationshipsNext[relationship._dotPath] || false }
};
const subname = relationship.name + ':' + type;
Expand Down Expand Up @@ -960,7 +984,7 @@ module.exports = {
const find = manager.find;

const options = {
find: find,
find,
builders: { relationships: withRelationshipsNext[relationship._dotPath] || false }
};

Expand Down Expand Up @@ -1195,7 +1219,7 @@ module.exports = {
const idsStorage = field.idsStorage;
const ids = await query.toDistinct(idsStorage);
const manager = self.apos.doc.getManager(field.withType);
const relationshipQuery = manager.find(query.req, { aposDocId: { $in: ids } }).project(manager.getRelationshipQueryBuilderChoicesProjection({ field: field }));
const relationshipQuery = manager.find(query.req, { aposDocId: { $in: ids } }).project(manager.getRelationshipQueryBuilderChoicesProjection({ field }));
if (field.builders) {
relationshipQuery.applyBuilders(field.builders);
}
Expand Down Expand Up @@ -1798,9 +1822,9 @@ module.exports = {
const values = await query.toDistinct(field.name);

const choices = _.map(values, function (value) {
const choice = _.find(allChoices, { value: value });
const choice = _.find(allChoices, { value });
return {
value: value,
value,
label: choice && (choice.label || value)
};
});
Expand Down Expand Up @@ -1983,7 +2007,7 @@ module.exports = {
fields[name] = component;
}
browserOptions.action = self.action;
browserOptions.components = { fields: fields };
browserOptions.components = { fields };
browserOptions.fieldMetadataComponents = self.fieldMetadataComponents;
browserOptions.customCellIndicators = self.uiManagerIndicators;
return browserOptions;
Expand Down
49 changes: 41 additions & 8 deletions modules/@apostrophecms/schema/lib/addFieldTypes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,17 @@ module.exports = (self) => {

self.addFieldType({
name: 'array',
async convert(req, field, data, destination, { fetchRelationships = true, ancestors = [] } = {}) {
async convert(
req,
field,
data,
destination,
{
fetchRelationships = true,
ancestors = [],
isParentVisible = true
} = {}
) {
const schema = field.schema;
data = data[field.name];
if (!Array.isArray(data)) {
Expand All @@ -779,7 +789,8 @@ module.exports = (self) => {
try {
const options = {
fetchRelationships,
ancestors: [ ...ancestors, destination ]
ancestors: [ ...ancestors, destination ],
isParentVisible
};
await self.convert(req, schema, datum, result, options);
} catch (e) {
Expand Down Expand Up @@ -861,9 +872,18 @@ module.exports = (self) => {

self.addFieldType({
name: 'object',
async convert(req, field, data, destination, {
fetchRelationships = true, ancestors = {}, doc = {}
} = {}) {
async convert(
req,
field,
data,
destination,
{
fetchRelationships = true,
ancestors = {},
isParentVisible = true,
doc = {}
} = {}
) {
data = data[field.name];
const schema = field.schema;
const errors = [];
Expand All @@ -873,7 +893,8 @@ module.exports = (self) => {
};
const options = {
fetchRelationships,
ancestors: [ ...ancestors, destination ]
ancestors: [ ...ancestors, destination ],
isParentVisible
};
if (data == null || typeof data !== 'object' || Array.isArray(data)) {
data = {};
Expand Down Expand Up @@ -963,8 +984,20 @@ module.exports = (self) => {
// properties is handled at a lower level in a beforeSave
// handler of the doc-type module.

async convert(req, field, data, destination, { fetchRelationships = true } = {}) {
const options = { fetchRelationships };
async convert(
req,
field,
data,
destination,
{
fetchRelationships = true,
isParentVisible = true
} = {}
) {
const options = {
fetchRelationships,
isParentVisible
};
const manager = self.apos.doc.getManager(field.withType);
if (!manager) {
throw Error('relationship with type ' + field.withType + ' unrecognized');
Expand Down
6 changes: 5 additions & 1 deletion modules/@apostrophecms/schema/ui/apos/mixins/AposInputChoicesMixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ const DEBOUNCE_TIMEOUT = 500;
export default {
data() {
return {
choices: []
choices: [],
enableValidate: false
};
},

Expand All @@ -33,6 +34,7 @@ export default {

methods: {
async getChoices() {
this.enableValidate = false;
if (typeof this.field.choices === 'string') {
const action = this.options.action;
const response = await apos.http.post(
Expand All @@ -58,10 +60,12 @@ export default {
}
},
updateChoices(choices) {
this.enableValidate = true;
this.choices = choices;
if (this.field.type === 'select') {
this.prependEmptyChoice();
}
this.validate(this.next);
},
prependEmptyChoice() {
// Using `hasOwn` here, not simply checking if `field.def` is truthy
Expand Down
3 changes: 3 additions & 0 deletions modules/@apostrophecms/schema/ui/apos/mixins/AposInputMixin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ export default {
// You must supply the validate method. It receives the
// internal representation used for editing (a string, for instance)
validateAndEmit () {
if (this.enableValidate === false) {
return;
}
// If the field is conditional and isn't shown, disregard any errors.
const error = this.conditionMet === false ? false : this.validate(this.next);

Expand Down

0 comments on commit 7a7f8a4

Please sign in to comment.