djaodjin / djaopsp

Practices Survey Platform - Assess against best practices assessment and improvement planning
BSD 2-Clause "Simplified" License
3 stars 6 forks source link

Load options for question types when app loads #360

Closed quiaro closed 7 months ago

quiaro commented 3 years ago

@smirolo what endpoint is responsible for providing the options for a question type?

smirolo commented 3 years ago

assessment answers. In the doc, the question_type is called default_metric and will be renamed question_type very soon.

quiaro commented 3 years ago

@smirolo I noticed there's an endpoint to get the options/units for a question: https://www.tspproject.org/docs/api/#ListUnits. Looks like this endpoint would return the options/units for all questions, correct? Or is it expected to get the slug as param e.g. /api/units/assessment?

Also, shouldn't the slug value correspond to the question type? i.e. in the response payload it would be:

"results": [
    {
      "slug": "assessment",
      "title": "assessment",
      "system": "enum",
      "choices": [
        {
          "rank": 1,
          "text": "mostly-yes",
          "descr": "Mostly yes"
        },
      ...

instead of:

      "slug": "assessments",
      "title": "assessments",
quiaro commented 3 years ago

I'm hoping this endpoint will provide me with the information that's currently hardcoded in /clients/web/src/config/questionFormTypes.js. Would you agree?

Btw, notice how the framework and assessment question types have a separate text and description fields besides the value field. Currently, it seems that the value field is named text.

smirolo commented 3 years ago

This issue is related to #358 .

There are no API end-point to retrieve a set of units from a question_type at this point. "GET /api/units" will retrieve all units information. What you have encoded as {value:..., text:...} is {slug: ..., title: ...}. For the units whose system == 'enum' (i.e. yes-no, assessment, framework), there is an additional field called choices with the options shown as radio buttons.

You can remove most of /clients/web/src/config/questionFormTypes.js, but you will need to keep the options field declared in FormQuestionEnergyConsumed, FormQuestionWaterConsumed, FormQuestionWasteGenerated. You will want to rename FormQuestionEmissionsGenerated to FormQuestionGHGEmissionsGenerated and add a FormQuestionGHGEmissionsScope3Generated.

quiaro commented 3 years ago

"GET /api/units" will retrieve all units [...] but you will need to keep the options field declared in FormQuestionEnergyConsumed, FormQuestionWaterConsumed, FormQuestionWasteGenerated.

Instead of keeping this options, I was thinking of mocking the response for GET /api/units with the information you've provided. Is that okay?

[...] there is an additional field called choices with the options shown as radio buttons.

As you might have seen in FormQuestionRadioRange and FormQuestionRadioLabeled, the UI currently assumes that the options for the radio buttons are made up of three properties: text, value and description. In https://www.tspproject.org/docs/api/#ListUnits, the properties for the choices are: rank, text and descr, where text seems to correspond to the UI value, descr seems to correspond to the UI text. Currently, rank doesn't have a matching property but I'll make sure to add it. On the other hand, the description field that the UI is using to provide additional information on the option does not seem to be included in the payload. Will you include it or will this additional clarifying information not be necessary anymore?

You will want to [...] add a FormQuestionGHGEmissionsScope3Generated.

What is this FormQuestionGHGEmissionsScope3Generated? How is it different to the current FormQuestionEmissionsGenerated?

smirolo commented 3 years ago

1/ Options are a UI concept and should stay as such (see #358).

2/ UI radio buttons to API choices (see example here vs. here):

rank is used to order the radio buttons (ex: initiating, progressing, transforming).

Unless there is some weird behavior on POST requests, we don't need a value field. As far as I can tell value = slugify(text) in all scenarios.

Note that the current implementation sends the text to the API when a choice is made.

We are trying to keep consistency across all APIs. I believe descr is used more often but I need to check.

3/ FormQuestionGHGEmissionsScope3Generated has an extra input relevance that FormQuestionEmissionsGenerated does not have.

quiaro commented 3 years ago

Unless there is some weird behavior on POST requests, we don't need a value field. As far as I can tell value = slugify(text) in all scenarios.

Note that the current implementation sends the text to the API when a choice is made.

In case the text ever needs to change, wouldn't it be better to decouple the option text (presentational) from its real value?