search

google / ground-platform

Ground hosted components: Web console, Cloud Functions, db config
http://groundplatform.org
Apache License 2.0
203 stars 86 forks source link

[Proto] Tasks in /job proto don't update on initial configuration #1911

Closed sufyanAbbasi closed 3 months ago

sufyanAbbasi commented 3 months ago

Describe the bug Not all tasks are being converted to proto for some reason.

To Reproduce Steps to reproduce the behavior:

  1. Sync repo to HEAD
  2. Run the local server with npm run start:local (localhost:5000)
  3. Create a new survey with all task types
  4. Check the Firebase emulator: http://localhost:4000/firestore/default/data/surveys/, see that the tasks under /jobs only have two entries and there are no task descriptions. Also see that the second entry has an index of -1, which is probably incorrect.

Expected behavior There should be 9 task protos for each task type.

Screenshots image image

gino-m commented 3 months ago

@rfontanarosa PTAL?

sufyanAbbasi commented 3 months ago

Ah, I think I have a picture for what's going on here... the trigger for updating /jobs is not happening when you hit "Continue" after the initial job configuration screen, it only triggers on survey title update because of the updateSurvey callback which does:

 await Promise.all(jobs.map(job => this.updateJob(surveyId, job)));

Or if we publish changes after an edit.

So in order to properly save the jobs in proto at the initial configuration, we have to trigger an updateSurvey when we hit Continue or something?

gino-m commented 3 months ago

Gentle ping @rfontanarosa

sufyanAbbasi commented 3 months ago

I think I found the bug! On "Continue" we run saveTasks() which doesn't call updateJob() which does the routine to serialize the tasks to proto. Maybe we can pull that logic in there?