guardian / typerighter

Even if you’re the right typer, couldn’t hurt to use Typerighter!
Apache License 2.0
276 stars 12 forks source link

Add publish button with server-side validation #335

Closed jonathonherbert closed 1 year ago

jonathonherbert commented 1 year ago

What does this change?

Adds a publish button with server-side validation. When a rule is changed, we ping the server to ask if the rule is publishable. If it's not, we get a tooltip with a list of reasons.

publish-validate

How to test

(Editing rules is a bit of a pain, now, in that when you hit 'update', you automatically navigate away from the current rule, and must re-open it to publish. Best to address in another PR, possibly by implementing real-time debounced updates.)

aracho1 commented 1 year ago

After I publish a rule, and try updating the rule in the same form (because the form does not close or refresh automatically after publishing), it gives me an error (but there is no error message).

image

To prevent this we could force the form to close after publishing (as I've observed that re-opening the form and updating the rule works as expected).

jonathonherbert commented 1 year ago

@aracho1 thanks for spotting this! It required a bit of a refactor to resolve, but it should be solved in https://github.com/guardian/typerighter/pull/339 – would it make sense to merge this and fix-forward in that PR?

jonathonherbert commented 1 year ago

@Fweddi agreed that we could leave this for the auto-saving story – I'm struggling to replicate, mind! Could you walk me through it?

Fweddi commented 1 year ago

@Fweddi agreed that we could leave this for the auto-saving story – I'm struggling to replicate, mind! Could you walk me through it?

I think what is happening is that when you hit the create button, it doesn't call the validation endpoint, and so retains the errors from the previous rule. In my case I was looking at an invalid live rule, and then hitting create.

The validation endpoint is only called when rule is truthy: https://github.com/guardian/typerighter/blob/1c1a847e5cabe06b710a7eeac403568fc10e552a/apps/rule-manager/client/src/ts/components/RuleForm.tsx#L59-L66

But when you hit the create button, you set the rule ID as undefined: https://github.com/guardian/typerighter/blob/1c1a847e5cabe06b710a7eeac403568fc10e552a/apps/rule-manager/client/src/ts/components/RulesTable.tsx#L210-L213 https://github.com/guardian/typerighter/blob/1c1a847e5cabe06b710a7eeac403568fc10e552a/apps/rule-manager/client/src/ts/components/RulesTable.tsx#L139-L142

Which in turn sets the rule as undefined: https://github.com/guardian/typerighter/blob/1c1a847e5cabe06b710a7eeac403568fc10e552a/apps/rule-manager/client/src/ts/components/hooks/useRule.ts#L116-L122

Which means rule is falsy and we don't validate when hitting the create button - instead, we keep the old publishing errors (as these are only ever set to undefined within the validate method itself).

jonathonherbert commented 1 year ago

Thanks for spotting! 8f8a92a should fix.