In-app upgrades

[Sean Colsen]

Hi team, especially Kriti, Brent, Dom, Pavish,

I just opened my In-app upgrades PR, and I’m not entirely sure how to approach review. This PR is tricky. It contains a lot of complexity but basically does nothing as-is. As I’ve mentioned, I can’t think of any way to fully test this working without making two live releases. I think it’s good-to-go, but I feel a bit uneasy about it, like I’m hurling something into a void. I’d love some extra eyes on it. I wrote up a pretty extensive PR description explaining how it all works.

• Kriti, it would be good for you to look over the screenshots in the PR description. There are a number of minor discrepancies vs Figma.

• Pavish, I’d like for you to give this a thorough review as soon as possible. Start by reading the PR description, and the diff will more sense.

• Brent and Dom, I’d like you to:

  • Read the PR description and confirm that the overall process is correct.
  • Consider if there are any ways we can manually test this work more easily. If this PR needs fixes, I’m loathe to end up a a feedback cycle that requires pushing one or two more live releases just to re-test.
  • Figure out who should fix the GET/POST issue with the upgrade endpoint, and make that fix ASAP so that we can cut our first live release to begin testing this stuff.

​

[Dominykas Mostauskis]

> Figure out who should fix the GET/POST issue with the upgrade endpoint, and make that fix ASAP so that we can cut our first live release to begin testing this stuff.

Is this the security concern regarding triggering the upgrade scan via GET instead of POST that we discussed Wednesday?

As I said then, I feel like we have more urgent things to work on and I think we should fix this some time after launch, instead of before.

[Dominykas Mostauskis]

> Consider if there are any ways we can manually test this work more easily. If this PR needs fixes, I’m loathe to end up a a feedback cycle that requires pushing one or two more live releases just to re-test.

We have a sync with Brent today. We'll try and work out what the steps for a mock release are. I'll assign #2485 to myself.

[Kriti Godey]

I reviewed the UI and commented on the issue.

Figure out who should fix the GET/POST issue with the upgrade endpoint, and make that fix ASAP so that we can cut our first live release to begin testing this stuff.

This should be a 30 sec fix. I don't think it's worth postponing it.

[Dominykas Mostauskis]

> This should be a 30 sec fix. I don't think it's worth postponing it.

I'd rather do it, than argue about it.

That said, programming the request rewriting in caddy is the easy part (though it's still orders of magnitude more than 30 seconds), but testing it is the actually laboursome part.

[Dominykas Mostauskis]

> Consider if there are any ways we can manually test this work more easily. If this PR needs fixes, I’m loathe to end up a a feedback cycle that requires pushing one or two more live releases just to re-test.

Made mock releases, check https://github.com/centerofci/mathesar/issues/2485.