Pro 8230 improve schema by ValJed · Pull Request #5044 · apostrophecms/apostrophe

ValJed · 2025-08-27T17:16:04Z

PRO-8230

Summary

Simplifies complex and not working code, see ticket for more info.

What are the specific steps to test this change?

Should work like before:
Cypress 🟢

What kind of change does this PR introduce?

Make sure the PR fulfills these requirements:

It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
The changelog is updated
Related documentation has been updated
Related tests have been updated

linear · 2025-08-27T17:16:07Z

PRO-8230 Improve AposSchema

ValJed · 2025-09-23T12:09:20Z

        :server-error="fields[field.name].serverError"
        :doc-id="docId"
        :generation="generation"
-        @update:model-value="updateNextAndEmit"


We now watch fieldState instead

ValJed · 2025-09-23T12:11:23Z

+
+      this.$emit('submit', {
+        name: this.subform.name,
+        values: this.docFields.data


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

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

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

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.

ValJed · 2025-09-23T12:12:11Z

    "uploadfs": "^1.25.1",
    "void-elements": "^3.1.0",
-    "vue": "^3.3.8",
+    "vue": "^3.5.20",


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

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.

ValJed · 2025-09-23T12:12:23Z

 * Fixes a bug in the login `uponSubmit` filter where a user could login without meeting the requirement.
-
-### Fixes
-


ValJed · 2025-09-23T12:15:28Z

            },
-            value: {
-              data: this.modelValue.data[item.name]
-            },


…date

haroun · 2025-09-24T16:05:14Z

+
+      this.$emit('submit', {
+        name: this.subform.name,
+        values: this.docFields.data


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

haroun · 2025-09-24T16:11:44Z

+
+      this.$emit('submit', {
+        name: this.subform.name,
+        values: this.docFields.data


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

haroun · 2025-09-24T16:24:58Z

+      this.next.fieldState = { ...this.fieldState };
+      this.$emit('update:model-value', this.next);


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

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 }; }

kept the spread

ValJed

Rollbacked what you asked for.

ValJed · 2025-09-25T14:19:08Z

+
+      this.$emit('submit', {
+        name: this.subform.name,
+        values: this.docFields.data


ETLaurent · 2025-10-07T13:37:22Z

 * 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.


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

* main: Pro 8230 improve schema (#5044)

ValJed self-assigned this Aug 27, 2025

ValJed marked this pull request as draft August 27, 2025 17:16

ValJed force-pushed the pro-8230-improve-schema branch 2 times, most recently from 6f2c5eb to e38acac Compare September 18, 2025 14:27

ValJed commented Sep 23, 2025

View reviewed changes

ValJed requested review from ETLaurent and haroun September 23, 2025 12:15

ValJed added 11 commits September 23, 2025 14:17

minor change

4dad71c

no updateNextAndEmit when inputs update

cceb2ea

upgrades vue version

a916437

simplifies AposSchema, improves perfs

b89bc3f

fixes null server errors for tests

01ce89b

removes debounce, puts back serverError in fields computed

799fb95

fixea AposSubform not resetting triggerValidation value

0114b5c

fixes AposSchema not resetting next.hasErrors to false before to vali…

36f7c5c

…date

fixes linter

3a902fb

adds changelog

c06c7f4

rollback server errors default

eeaf472

ValJed force-pushed the pro-8230-improve-schema branch from b009bad to eeaf472 Compare September 23, 2025 12:17

ValJed marked this pull request as ready for review September 23, 2025 12:18

haroun suggested changes Sep 24, 2025

View reviewed changes

ValJed added 2 commits September 25, 2025 16:09

feedback: spread next before to emit next from schema (like it was)

e2d9fda

feedback: rollback weird triggerValidation not being resetted

1451de3

ValJed commented Sep 25, 2025

View reviewed changes

ValJed requested a review from haroun September 25, 2025 14:19

ETLaurent previously approved these changes Oct 7, 2025

View reviewed changes

haroun previously approved these changes Oct 8, 2025

View reviewed changes

ValJed dismissed stale reviews from haroun and ETLaurent via 498b9e9 October 8, 2025 08:38

ValJed force-pushed the pro-8230-improve-schema branch from 498b9e9 to 1451de3 Compare October 8, 2025 08:40

Merge remote-tracking branch 'origin/main' into pro-8230-improve-schema

9e72c4d

ValJed requested review from ETLaurent and haroun October 8, 2025 08:41

haroun approved these changes Oct 8, 2025

View reviewed changes

ValJed merged commit 95626fe into main Oct 8, 2025
9 checks passed

ValJed deleted the pro-8230-improve-schema branch October 8, 2025 09:52

haroun added a commit that referenced this pull request Oct 9, 2025

Merge branch 'main' into pro-8255-manager-locale-switcher

80a96aa

* main: Pro 8230 improve schema (#5044)

		* Fixes a bug in the login `uponSubmit` filter where a user could login without meeting the requirement.

		### Fixes

		this.next.fieldState = { ...this.fieldState };
		this.$emit('update:model-value', this.next);

Conversation

ValJed commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What are the specific steps to test this change?

What kind of change does this PR introduce?

Make sure the PR fulfills these requirements:

Uh oh!

linear Bot commented Aug 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValJed Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValJed Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValJed Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValJed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ValJed commented Aug 27, 2025 •

edited

Loading

ValJed Sep 23, 2025 •

edited

Loading

ValJed Sep 23, 2025 •

edited

Loading

ValJed Sep 23, 2025 •

edited

Loading