djaodjin / djaopsp

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

Update current practices section to use latest API #358

Closed quiaro closed 10 months ago

quiaro commented 3 years ago

@smirolo in the answers response payload, there used to be a collected_by field and a created_at field, but I don't see them anymore in the sample response https://www.tspproject.org/docs/api/#RetrieveSampleAnswers. Have they been removed or are they missing from the sample response?

quiaro commented 3 years ago

Also, I believe you mentioned adding a required property. Should it also be present in the response?

quiaro commented 3 years ago

@smirolo I was assuming the payload for https://www.tspproject.org/docs/api/#RetrievePageElementTree, would be similar to:

{
    path: null,
    title: 'Assessment',
    indent: 2,
  },
  {
    path:
      '/metal/boxes-and-enclosures/management-basics/assessment/the-assessment-process-is-rigorous',
    title: 'The assessment process is rigorous and thorough*',
    default_unit: 'freetext',
    indent: 3,
    environmental_value: 1,
    business_value: 1,
    profitability: 1,
    implementation_ease: 1,
    avg_value: 1,
  },

Is this still correct or has any of this changed? Thanks!

quiaro commented 3 years ago

@smirolo Per https://www.tspproject.org/docs/api/#RetrieveSampleAnswers, how would the FE know what the question type is (for rendering purposes) if there's no question type information anymore, only units and default_units. Specifically, how would the FE know how to render the following question types:

smirolo commented 3 years ago

You should GET https://www.tspproject.org/docs/api/#RetrieveSampleAnswers. This will return the questions (question type is in results[idx].question.question_type). GET https://www.tspproject.org/docs/api/#RetrievePageElementTree will return practice text content and intrinsic practices values. Both API calls have a similar structure for the output.

quiaro commented 3 years ago

I noticed in https://www.tspproject.org/docs/api/#RetrieveAssessmentAnswers that the question_type value for an assessment question is radio , but up until now the question types that I know of are: 'assessment' 'comments' 'ghg-emissions-generated' 'employee-counted' 'energy-consumed' 'framework' 'freetext' 'relevance' 'revenue-generated' 'waste-generated' 'water-consumed' 'yes-no'

What are the possible question_type values?

smirolo commented 3 years ago

Based on my reading of the Vue client code and _assess_row.html template, here the component hierarchy:

Implementation Notes:

quiaro commented 3 years ago

@smirolo Thanks for the response.

To recap, you're suggesting that the new question types will be:

Correct?

In the case of enum, why would the API overload the default_unit property with question_type information? This may be easily misinterpreted. Why wouldn't different question types be created instead (assessment, framework, yes-no) and have the /api/units endpoint be used to retrieve options/units for the question types (in which case, /api/options may be a better name).

  • FormQuestionBinary and FormQuestionRadio are equivalent except for the expected comment in FormQuestionBinary, while it is very much optional in FormQuestionRadio. [...] Here we can also decide to instantiate one or the other with the question.default_unit == 'yes=no'.

I would prefer to keep them as separate UI components (though they use the same model -FormQuestionRadio) given that one expects more information than the other. If they were to be merged, this extra logic might needlessly complicate the implementation of the component.

  • Targets will be stored with question_type == 'freetext'.

This is currently being done in the Vue client.

Maybe rename [FormSingleTarget] to FormQuestionSingleTarget to match naming convention.

I think this would be misleading since FormSingleTarget has an entirely different set of requirements.

  • I am not sure the difference between FormQuestionTextareaControlled and FormQuestionTextarea.

FormQuestionTextareaControlled was created to delegate the responsibility of question validation to the parent component. As I mentioned previously, targets are currently using the question type value of freetext. If these target questions were to be shown in the assessment, they would use the FormQuestionTextarea component, which is a thin wrapper (that doesn't require validation) around a FormQuestionTextareaControlled component. However, in the context of the target questionnaire, these target questions must be validated before submission by their parent component and thus use the FormQuestionTextareaControlled instead.

smirolo commented 3 years ago

OK. So I rename question_type to ui_hint so it is clear the backend does not use that information at all. It is like a tag to help the client to decide the appropriate UI element to display to enter the answer/measurement.

The UI hints are:

They should readily match to the FormQuestionX component.

The /api/units end point is to retrieve units of measurements (ex: meters, kilograms or a qualitative enum like assessment value). There is no API for 'options' in the sense used in the Vue components. I thought about it and it seems there are too many use cases where we might want to present metric options in Europe and imperial options in the USA. Thus decision of the options to present the user should be left to the UI component.

The default_unit now returns a dictionary that match /api/units/{unit.slug} because for the framework case we need descriptions which are tied to both the question and unit.