PC-577: add solar pv question #312
Conversation
|
Will do Miro Changes soon, and tick the box when done. |
| { | ||
| SolarElectricPanels.Yes => true, | ||
| SolarElectricPanels.No => false, | ||
| SolarElectricPanels.DoNotKnow => null, |
There was a problem hiding this comment.
| SolarElectricPanels.DoNotKnow => null, | |
| SolarElectricPanels.DoNotKnow => false, |
We should be sending false if the user answers "I don't know": doesn't look like that's happening at the moment? Looks like this is the place for it?
There was a problem hiding this comment.
Null and False lead to the same outcome with the API. I've left it this way because I think when we get the hint data, we want to override the user's option on a "don't know" with the open EPC information.
At least that was my understanding of the documents on the ticket.
There was a problem hiding this comment.
I'm not sure I follow: I can see their change doc has it as a non-nullable boolean, and I'm looking at the requirements:
User response will be used to populate a pv_panel field in the
This is a boolean true or false field (see API document below)
If the user answers “Yes”, we send true
If the user answers “No” or “I don’t know” , we send false
It should be added to the BreRequest object that is sent in the BRE recommendation request (see BreApi class)
It could be worth double checking with BRE what they're expecting from us here
There was a problem hiding this comment.
Ah! No that's fair, I'll change it over. I was looking at the attached document's open epc thing
There was a problem hiding this comment.
this change has been made. left a fallback 'null' in the case of no solar pv answer
| if (entryPoint is not null) | ||
| { | ||
| return QuestionFlowStep.AnswerSummary; | ||
| } |
There was a problem hiding this comment.
What does this mean has happened? User's changing their answers? Can that be made any clearer?
There was a problem hiding this comment.
- Yes, the user is going back to edit
- I've missed some logic here, for the roof construction question
- I've also just realised I've made a mistake with this question placement, because of that. The question before this has follow-on questions (loft questions), and because of the way routing works on FWTS, it will always push the user through the Solar Panels question again to answer the loft questions if they go back and change their answer.
There was a problem hiding this comment.
I interpreted the ticket as "directly after" the roof construction question, not just after, which was my mistake.
There was a problem hiding this comment.
spoke with glenn yesterday, we agreed to move the solar pv question to the end of the roof questions flow, which makes the most sense. its also GDS compliant, since putting a non conditional question between conditional ones can cause issues with the change page asking questions it doesnt need to
| migrationBuilder.AddColumn<int>( | ||
| name: "SolarElectricPanels", | ||
| table: "PropertyData", | ||
| type: "integer", |
There was a problem hiding this comment.
integer is a bit of a surprise? Why's that?
There was a problem hiding this comment.
It's the underlying type for the enum, the index of the value.
From PropertyData.cs:
public SolarElectricPanels? SolarElectricPanels { get; set; }
if user was midway through form when updated released, for instance. stops summary page crash
as requested on ticket. leave a null option in the case of no solar pv option (previous behaviour)
samyou-softwire
force-pushed
the
PC-577-Add-Solar-PV-question
branch
from
1000c41 to
c1eefd1
Compare
There was a problem hiding this comment.
Don't think we want these (should probably be gitignored)
There was a problem hiding this comment.
have gitignored these now
this has now been done |
Link to Jira ticket
Description
Adding the Solar PV question to the journey & adding the response to the BRE API request.
Checklist
Screenshots