Skip to content

fix: run existing-installation migrations before installer questions#814

Open
anonymoususer72041 wants to merge 5 commits into
opencats:masterfrom
anonymoususer72041:fix/existing-installation-migration-order
Open

fix: run existing-installation migrations before installer questions#814
anonymoususer72041 wants to merge 5 commits into
opencats:masterfrom
anonymoususer72041:fix/existing-installation-migration-order

Conversation

@anonymoususer72041

Copy link
Copy Markdown
Contributor

This PR updates the installer flow for existing OpenCATS installations so that schema maintenance and migrations run immediately after the user chooses to continue with the existing installation.

Previously, the existing-installation path continued directly to the later installer questions, such as resume parsing options. That meant those questions could be reached before pending maintenance or schema migrations had been processed. The updated flow routes existing installations through the existing maintenance mechanism first and then resumes the installer at the resume parsing step.

The normal maintenance step later in the installer flow is skipped for this same existing-installation upgrade session once the earlier maintenance run has completed. This avoids running the maintenance flow twice while preserving the existing post-maintenance continuation behavior.

Fresh installations continue to use the existing installer flow. The reinstall path also clears the existing-upgrade maintenance session flags so that a previous existing-installation attempt cannot affect a reinstall.

The implementation reuses the existing installer maintenance infrastructure and does not introduce separate schema migration logic.

@anonymoususer72041 anonymoususer72041 requested a review from RussH June 23, 2026 13:29
@anonymoususer72041 anonymoususer72041 force-pushed the fix/existing-installation-migration-order branch from 5edcf25 to e4341b0 Compare June 23, 2026 13:30
@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

@RussH I thought a little about the current situation and decided to create this PR with a temporary resolution, even if I plan to remove the installer for existing migrations after #803 got merged.

@anonymoususer72041 anonymoususer72041 added this to the 0.11.0 milestone Jun 24, 2026

@RussH RussH left a comment

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.

These are some pretty fundamental installer-flow changes, so I’d like a little bit of CI coverage here if possible.

I don’t think we need a huge installer integration suite for this PR, but one or two focused tests would help a lot:

  • existing-install path runs the upgrade/migration action before continuing to the installer questions
  • migration failure stops the continuation path instead of carrying on as if the upgrade succeeded

If we already have a way to test installer JS, a small test around installMaintNextAction being used and reset would also be helpful.

Overall I like the direction — I just want to make sure this flow doesn’t regress later, since installer state/action changes can be a bit hard to catch manually.

@anonymoususer72041

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense.

I added a small follow-up instead of a broader installer integration suite, since the existing-installation installer path is intended to be removed after #803.

The update adds focused regression coverage for the flow introduced here:

  • the existing-install path routes into the maintenance flow before continuing to later installer questions
  • the post-maintenance continuation resumes at the resume parsing step
  • the later normal maintenance step is skipped once for the same existing-installation upgrade session and clears the temporary session flags
  • reset/reinstall clears the temporary existing-upgrade maintenance flags

I also added a small guard in the installer JavaScript so the maintenance continuation is only used after a successful HTTP response. The accompanying test checks that Installpage_maint() requires http.status == 200 before continuing via installMaintNextAction, and that the continuation action is reset afterwards.

This is intentionally still a focused source-level regression test rather than a full installer/browser/database integration test, to keep the change proportional to the temporary nature of this path.

@anonymoususer72041 anonymoususer72041 requested a review from RussH June 24, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants