teams: smoother survey demographics handling (fixes #9983)#9984
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b14d20bea8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds an optional demographics step (birth year + gender) after completing a public survey, and sends the sanitized demographics payload through to the gateway so it can be stored with the public submission.
Changes:
- Frontend: introduce a post-survey demographics UI step and include demographics in the public survey submission payload.
- Frontend: refactor inline component styles into a dedicated SCSS file.
- Gateway: accept and sanitize optional
userdemographics fields (age,gender) when creating public survey submissions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/app/exams/public-surveys/public-surveys.service.ts | Extends submitSurvey to optionally include demographics (user) in the POST body. |
| src/app/exams/public-surveys/public-survey.component.ts | Adds demographics form + navigation flow; builds optional demographics payload for submission. |
| src/app/exams/public-surveys/public-survey.component.scss | New stylesheet for state views and demographics form layout. |
| src/app/exams/public-surveys/public-survey.component.html | Adds demographics step UI; updates navigation buttons to route to demographics before submission. |
| gateway/src/modules/public/services/surveys.service.ts | Extends payload typing and sanitizes stored submission user demographics fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f88084cce4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Tried a couple of times, but I'm not seeing the info from that form being reflected in the data export. The last two entries were made with this branch. 
UPDATE: I see there were changes to the planet gateway service. Using the image from this branch fixes that issue. |
paulbert
left a comment
paulbert
left a comment
There was a problem hiding this comment.
Looks like it's working as expected, nice work!
I've got some comments on error handling, formatting, and code quality, but I'll go ahead and approve.
| const age = user.age; | ||
| const gender = typeof user.gender === 'string' ? user.gender.trim() : ''; | ||
|
|
||
| if (typeof age === 'number' && Number.isInteger(age) && age >= 0 && age <= 130) { |
There was a problem hiding this comment.
Should at least log if not throw something if age is in the payload and not in the correct range.
I think in the frontend this is limited by a minimum year, not maximum age as well. Not ideal to have the mismatch here as in a few years a valid form value will be removed here. Also frontend rejects age 0, but this would accept that.
| sanitizedUser.age = age; | ||
| } | ||
|
|
||
| if (gender === 'male' || gender === 'female') { |
There was a problem hiding this comment.
Same as above, log or throw an error if the value exists and is invalid.
| </div> | ||
| </planet-exams-question-frame> | ||
|
|
||
| <div *ngIf="survey && showDemographics && !isSubmitted" class="state-view"> |
There was a problem hiding this comment.
Is there a reason why this form has the fields flipped from the "Record Survey" version?
Generally not ideal that this is duplicated from that, but I see that is also a bit entangled with the UsersUpdateComponent so I think this is OK.
| readonly answer = new FormControl<ExamAnswerValue>(null, { validators: examAnswerValidator }); | ||
| readonly demographicsForm = this.fb.group({ | ||
| birthYear: this.fb.control<number | null>(null, [ | ||
| Validators.min(1900), |
There was a problem hiding this comment.
It would be cleaner with the backend to match this to the current year minus 130 rather than a minimum year.
| user.gender = gender; | ||
| } | ||
|
|
||
| return Object.keys(user).length > 0 ? user : undefined; |
There was a problem hiding this comment.
I think you should just return the user here. There's no check from here to the database on whether the user object is undefined other than to turn it into an empty object if it is.
By explicitly setting it to undefined if it's an empty object, you're setting an expectation that that matters somewhere in the application.
dogi
changed the title
Fixes #9983
Add the optional demographics information form at the end of public surveys