Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,14 @@

* Fixes a bug in the login `uponSubmit` filter where a user could login without meeting the requirement.
* Fixes pieces filters when values from optional fields are falsy.

### Fixes

Copy link
Copy Markdown
Contributor Author

@ValJed ValJed Sep 23, 2025

Choose a reason for hiding this comment

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

duplicated

* Resolve inline image URLs correctly when in edit mode and not in the default locale.

### Changes

* Redirects to URLs containing accent marks and other non-ascii characters now behave as expected with Astro. Pre-encoding the URLs exactly the way `res.redirect` would before passing them to Astro prevents an error in Astro and allows the redirect to succeed.
* Removes the non-functional `uniqueUsername` route from the `user` module
* Updated dependencies to address deprecation warnings.

* Refactors complex logic from `AposSchema` that handle data updates to simplifies it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

will need to be placed at the top after the merge of main


## 4.21.0 (2025-09-03)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export default {
async handleSaveError(e, { fallback }) {
// eslint-disable-next-line no-console
console.error(e);
if (e.body && e.body.data && e.body.data.errors) {
if (e.body?.data?.errors) {
const serverErrors = {};
let first;
e.body.data.errors.forEach(e => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
:server-error="fields[field.name].serverError"
:doc-id="docId"
:generation="generation"
@update:model-value="updateNextAndEmit"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We now watch fieldState instead

@update-doc-data="onUpdateDocData"
@validate="emitValidate()"
/>
Expand Down
53 changes: 11 additions & 42 deletions modules/@apostrophecms/schema/ui/apos/logic/AposSchema.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { detectFieldChange } from 'Modules/@apostrophecms/schema/lib/detectChange';
Comment thread
haroun marked this conversation as resolved.
import { getConditionTypesObject } from '../lib/conditionalFields';

export default {
Expand Down Expand Up @@ -130,9 +129,6 @@ export default {
...item,
required
},
value: {
data: this.modelValue.data[item.name]
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unused

serverError: this.serverErrors && this.serverErrors[item.name],
modifiers: this.computeModifiers(item)
}
Expand Down Expand Up @@ -167,6 +163,13 @@ export default {
}
},
watch: {
fieldState: {
deep: 2,
handler() {
this.updateNextAndEmit();
},
flush: 'post'
},
schema() {
this.populateDocData();
},
Expand Down Expand Up @@ -211,6 +214,7 @@ export default {
created() {
this.populateDocData();
},

methods: {
emitValidate() {
this.$emit('validate');
Expand Down Expand Up @@ -269,53 +273,18 @@ export default {
if (!this.schemaReady) {
return;
}
const oldHasErrors = this.next.hasErrors;
// destructure these for non-linked comparison
const oldFieldState = { ...this.next.fieldState };
const newFieldState = { ...this.fieldState };

let changeFound = false;

this.next.hasErrors = false;
this.next.fieldState = { ...this.fieldState };

this.schema
.filter(field => this.displayComponent(field))
.forEach(field => {
if (this.fieldState[field.name].error) {
this.next.hasErrors = true;
}
// This simply check if a field has changed since it has been
// instantiated
if (
this.fieldState[field.name].data !== undefined &&
detectFieldChange(
field,
this.next.data[field.name],
this.fieldState[field.name].data
)
) {
changeFound = true;

// fieldState never gets the relationships postprocessed data
// that's why it gets seen as different than next all the time
this.next.data[field.name] = this.fieldState[field.name].data;
} else {
this.next.data[field.name] = this.modelValue.data[field.name];
}
this.next.data[field.name] = this.fieldState[field.name].data;
});
if (
oldHasErrors !== this.next.hasErrors ||
oldFieldState !== newFieldState
) {
// Otherwise the save button may never unlock
changeFound = true;
}

if (changeFound) {
// ... removes need for deep watch at parent level
this.$emit('update:model-value', { ...this.next });
}
this.next.fieldState = { ...this.fieldState };
this.$emit('update:model-value', this.next);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are those 2 lines equivalent to what we had before with ...this.next?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This line was already there:

      this.next.fieldState = { ...this.fieldState };

For the emit I removed the spread because it's already done in the parent that spreads value.data and is useless I think. (we can keep it if you have a doubt).

    updateDocFields(value) {
      this.updateFieldErrors(value.fieldState);
      this.docFields.data = {
        ...this.docFields.data,
        ...value.data
      };
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

kept the spread

},
displayComponent({ name, hidden = false }) {
if (hidden === true) {
Expand Down
19 changes: 10 additions & 9 deletions modules/@apostrophecms/schema/ui/apos/logic/AposSubform.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,16 @@ export default {
},
async submit() {
this.triggerValidation = true;
this.$nextTick(async () => {
if (this.docFields.hasErrors) {
this.triggerValidation = false;
return;
}
this.$emit('submit', {
name: this.subform.name,
values: this.docFields.data
});
await this.$nextTick();
this.triggerValidation = false;

if (this.docFields.hasErrors) {
return;
}

this.$emit('submit', {
name: this.subform.name,
values: this.docFields.data
Copy link
Copy Markdown
Contributor Author

@ValJed ValJed Sep 23, 2025

Choose a reason for hiding this comment

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

Made this change because the old version wasn't resetting triggerValidation to false when no errors.(Probably the modal closes anyway but cleaner).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Initially we never set triggerValidation back to false if there was no errors. What about all the other places?

Image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm afraid this might say I can submit the form multiple times too if the form is still open.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It wasn't done here I think because the modal was closed right after so not a big deal.
In the other locations we normally set it back to false after having awaited the nextTick.
You mean resetting the value would allow to submit the form with errors ? I don't think so, triggerValidation and errors are independent, But I agree it's pretty weird that we reset it only if no errors.. it should behave like the function I think:

    triggerValidate() {
      this.triggerValidation = true;
      this.$nextTick(async () => {
        this.triggerValidation = false;
      });
    }

This system is far from ideal thought.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rollbacked

});
},
async cancel() {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
"underscore.string": "^3.3.4",
"uploadfs": "^1.25.1",
"void-elements": "^3.1.0",
"vue": "^3.3.8",
"vue": "^3.5.20",
Copy link
Copy Markdown
Contributor Author

@ValJed ValJed Sep 23, 2025

Choose a reason for hiding this comment

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

I updated for the vue watcher deep feature to be available. Tests are green.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

btw our regression tests always do the equivalent of "npm update" (or a fresh install without package lock), so they would not have passed anyway before this if they were incompatible with latest vue 3.x.

Your change does make sense however, to force the update of vue to the version you need when clients update apostrophe.

"vue-advanced-cropper": "^2.8.8",
"vue-loader": "^17.1.0",
"vue-style-loader": "^4.1.3",
Expand Down