irfanpule / django-form-surveys

Django form survey is an application Django to easier create form survey and easy integrated for your project.
MIT License
56 stars 19 forks source link

Add option to change number of ratings #71

Closed MartinK84 closed 8 months ago

MartinK84 commented 8 months ago

This rather complicated pull request adds the possibility to change the number of ratings used by adding a new "Number of ratings" property to rating questions: image

Internally it simply stores the number of ratings in the choices property of the question model as this was previously not used for rating questions. Simply reusing this property is IMO much simpler than creating a new model property just for the ratings.

The number of ratings displayed is then dynamically updated everywhere: image image

For future extensions one could think about storing json() encoded meta information in the choices field to bundle together more custom options - but I think that's a rather big change for future updates.

irfanpule commented 8 months ago

Hi @MartinK84 Thank for your PR. That is great update. I agree with you if use storing json() can be big change for the feature, so until now I don't think to change it. But I have plan for use storing json() to save option validation or anything each fields.

MartinK84 commented 8 months ago

I have also added another commit that changes the unit test to use RatingValidator() in case we stick with this (see discussion above), somehow missed this before, sorry

MartinK84 commented 8 months ago

Regarding the long term change to store custom info per question as json(), I think it would be not that difficult to implement but would break backward compatibility (or be a super huge headache with lots of code to keep it compatibly), so old installations/databases won't work anymore.

In the long run, however, I think this would be super useful as one could add a lot more options to the individual questions.

For example for the rating questions I would like to add:

For the ratings one could probably implement this by using the choices property of the model and just dump json() in there, but in the long run a more generic solution would be great :-)

irfanpule commented 8 months ago

I have also added another commit that changes the unit test to use RatingValidator() in case we stick with this (see discussion above), somehow missed this before, sorry

No worries, I missed too. Thank you

irfanpule commented 8 months ago

Regarding the long term change to store custom info per question as json(), I think it would be not that difficult to implement but would break backward compatibility (or be a super huge headache with lots of code to keep it compatibly), so old installations/databases won't work anymore.

In the long run, however, I think this would be super useful as one could add a lot more options to the individual questions.

For example for the rating questions I would like to add:

  • the possibility to chose between different styles of the star (e.g. star vs. bullet)
  • option to display labels at the lower and upper star (e.g. "1" next to the first star and "5" next to the last star)
  • option to display custom labels next to the stars, same as before but with user defined custom text - so that one could write "poor" next to the first star or "excellent" next to the last star. That would be a great addition to make it clearer what is rated (which might different depending on the question).

For the ratings one could probably implement this by using the choices property of the model and just dump json() in there, but in the long run a more generic solution would be great :-)

Initially I created this module trying to keep it simple. Prioritize compatibility of all databases. But after developing until now, I also think about this. Maybe to maintain compatibility the model doesn't need to be initialized as a json field, maybe it could be a string of json stored in a textfield. For example like this:

class Question(BaseModel):
    ....
    extra_data = models.TextField()  # use to save custom property

The standard JSON data stored can vary depending on the field type requirements and then we don't have to change base model structure.

MartinK84 commented 8 months ago

Initially I created this module trying to keep it simple. Prioritize compatibility of all databases. But after developing until now, I also think about this. Maybe to maintain compatibility the model doesn't need to be initialized as a json field, maybe it could be a string of json stored in a textfield. For example like this:

class Question(BaseModel):
    ....
    extra_data = models.TextField()  # use to save custom property

The standard JSON data stored can vary depending on the field type requirements and then we don't have to change base model structure.

Just using a TextField() is also what I had in mind, this is super simple and you just have to slap a json.loads() and json.dumps() around it in code and then any type of question can use it in whatever way it likes to.