Start error reporting

[Ladsgroup]

This is basic error reporting, the message component is not yet released
so it can't be used but most of the work of showing the user is done
now.

Bug: T266788
Bug: T266787

[Ladsgroup]

Tests needed

[Ladsgroup]

Tests needed

Test added.

[micgro42]

🤔 This adds the store infrastructure, but is not actually using it? Or am I missing something?

[Ladsgroup]

thinking This adds the store infrastructure, but is not actually using it? Or am I missing something?

It does. It's a computed property passed around using mapState(). You can check out the PR and try it yourself (just don't write anything and click on Run query)

[micgro42]

thinking This adds the store infrastructure, but is not actually using it? Or am I missing something?

It does. It's a computed property passed around using mapState(). You can check out the PR and try it yourself (just don't write anything and click on Run query)

This is very strange magic. That helper just plainly assumes that setErrors actions and such exist? 🙃

But it works 🤷

[micgro42]

That being said, this still needs more work for setting the validation/error states for each input field.

[Ladsgroup]

thinking This adds the store infrastructure, but is not actually using it? Or am I missing something?

It does. It's a computed property passed around using mapState(). You can check out the PR and try it yourself (just don't write anything and click on Run query)

This is very strange magic. That helper just plainly assumes that setErrors actions and such exist? upside_down_face

But it works shrug

The validate method dispatches setErrors: this.$store.dispatch( 'setErrors', [] );

That being said, this still needs more work for setting the validation/error states for each input field.

I think that's different in nature and needs to be done in a follow up (I will do it, just don't want to make this PR super big)

[micgro42]

Hey, with the recently merged PRs, some nasty merge conflicts were introduced. I rebased your PR (since I also broke it :p). Please check that everything is still alright

[micgro42]

thinking This adds the store infrastructure, but is not actually using it? Or am I missing something?

It does. It's a computed property passed around using mapState(). You can check out the PR and try it yourself (just don't write anything and click on Run query)

Mh, but that could do just as well without the store and only with a local state called error, since the validation is done in a method and not an action. Though, I think it works for this pull request. We can move it when dealing with setting the error states on the individual inputs in the next PR.

[micgro42]

this.errors = [];
this.$store.dispatch( 'setErrors', [] );

Those two lines together are strange. Since we are mapping the state to errors, shouldn't be one enough?

[Ladsgroup]

IIRC, this is an issue with proxies in Vue2 (setting arrays) and can be removed in Vue3.

[micgro42]

We should look into migrating to Vue3. Probably, better sooner than later.

[Ladsgroup]

I think it was a deliberate choice to go with vue2, I think it was because of consistency across the whole wmde codebase (and we can't migrate mediawiki micron frontends yet). Jakob knows the details.

[Ladsgroup]

thinking This adds the store infrastructure, but is not actually using it? Or am I missing something?

It does. It's a computed property passed around using mapState(). You can check out the PR and try it yourself (just don't write anything and click on Run query)

Mh, but that could do just as well without the store and only with a local state called error, since the validation is done in a method and not an action. Though, I think it works for this pull request. We can move it when dealing with setting the error states on the individual inputs in the next PR.

My reasoning is that with this app getting bigger and bigger (and the need to introduce more components), we certainly need the error state into Vuex

[micgro42]

thinking This adds the store infrastructure, but is not actually using it? Or am I missing something?

It does. It's a computed property passed around using mapState(). You can check out the PR and try it yourself (just don't write anything and click on Run query)

Mh, but that could do just as well without the store and only with a local state called error, since the validation is done in a method and not an action. Though, I think it works for this pull request. We can move it when dealing with setting the error states on the individual inputs in the next PR.

My reasoning is that with this app getting bigger and bigger (and the need to introduce more components), we certainly need the error state into Vuex

Yeah, and that is probably true for the actual validation code as well, that should move into an action. But we can do that later.