Fixed new survey import concurrency issue

[SteRiccio]

No description provided.

[Copilot]

Pull request overview

This PR addresses a concurrency issue in survey creation/import where holding a DB transaction open while running schema migrations could exhaust the connection pool and hang the server. It refactors SurveyManager.insertSurvey and SurveyManager.importSurvey to avoid wrapping schema migration inside a held-open transaction, and updates the create-survey flow to return a job (aligning with the UI job-monitor pattern), with added regression coverage.

Changes:

• Refactor survey create/import manager functions to run schema migration outside an open transaction and add cleanup on failure.
• Change the /survey create endpoint to enqueue a new SurveyCreateJob (instead of returning a created survey synchronously).
• Add integration regression tests for concurrent survey create/import; update E2E survey creation to wait for the job modal flow.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/integration/tests/001surveyIntegrationtest.js	Adds new integration test cases for concurrent survey operations.
test/integration/tests/_survey/surveyTest.js	Implements concurrent create/import regression tests using Promise.all.
test/e2e/tests/_surveyCreate/index.js	Updates E2E flow to wait for job completion modal for survey creation/clone.
server/modules/survey/service/surveyService.js	Adds startCreateSurveyJob to enqueue create-survey as a job.
server/modules/survey/service/surveyCreateJob.js	Introduces a new job class for survey creation and returns surveyId as job result.
server/modules/survey/manager/surveyManager.js	Refactors insertSurvey/importSurvey to avoid running schema migration under a held-open transaction; adds cleanup on error.
server/modules/survey/api/surveyApi.js	Changes create survey API to return a job for new surveys (consistent with clone path).
server/job/jobCreator.js	Registers SurveyCreateJob so it can be instantiated by the job system.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[Copilot]

Pull request overview

Copilot reviewed 29 out of 31 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

test/load/surveyImportStressTest.ts:100

• Similarly, the lastKnown* fields should be updated with ?? rather than || to avoid skipping legitimate falsy values.

      lastKnownSurveyId = job.surveyId || lastKnownSurveyId
      lastKnownErrors = job.errors || lastKnownErrors
      lastKnownResult = job.result || lastKnownResult

test/load/surveyImportStressTest.ts:395

• import.meta.main is not a Node.js standard entrypoint check and (when undefined) will prevent main() from running when this script is executed via node .... This will make yarn test:load appear to do nothing.

if (import.meta.main) {
  main().catch((error) => {
    console.error('Stress test failed to run:', error)
    process.exitCode = 1
  })

test/load/surveyImportStressTest.ts:94

• Backfilling surveyId/errors/result using || will treat valid falsy values (e.g. empty arrays/objects) as “missing” and incorrectly fall back to stale values from a previous poll. Using nullish coalescing avoids that.

This issue also appears on line 98 of the same file.

        surveyId: job.surveyId || lastKnownSurveyId,
        errors: job.errors || lastKnownErrors,
        result: job.result || lastKnownResult,

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud