Skip to content
This repository has been archived by the owner on Feb 13, 2022. It is now read-only.

removed pagination from PollsViaSurveys #103

Merged
merged 8 commits into from
Apr 3, 2018

Conversation

sewagodimo
Copy link
Contributor

@sewagodimo sewagodimo commented Mar 27, 2018

When configuring Polls via Surveys via Display Question Directly, results in an Internal Server Error when configuring Skip Logic fields. The solution was to remove pagination from polls that are created via surveys

Copy link
Contributor

@smarzban smarzban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍨

@sewagodimo
Copy link
Contributor Author

@nathanbegbie could you please check that my tests are are written up to standard

@nathanbegbie
Copy link
Contributor

nathanbegbie commented Mar 28, 2018

Our current tests are by no means perfect and this is a good opportunity to follow better patterns and leave the code better than we found it.

Couple of things we can do to improve the code:

  1. Move these tests out of models.py and into views.py. The fact that we're using request from the server usually means that we're testing the view, not the model, and that this is more of an integration test than a unit test.
  2. Be careful when using 'helper' functions in tests. Always check to see how we can make these tests reuable - in fact you'll see that there are plenty of opportunities for making the testing code more reusable. Have a look at https://github.com/praekelt/molo.surveys/blob/develop/molo/surveys/tests/test_models.py#L371 and https://github.com/praekelt/molo.surveys/blob/develop/molo/surveys/tests/test_models.py#L386 -> these are functions that seem to have some overlap with your test functions. Move the functions into tests/base.py and refactor them to make things more reusable.
  3. Strip out the bits you don't need -> have a look at your setUp.py method and make sure that what is used is needed. Comment out lines and see if your tests still pass. Also have a look at MoloTestCaseMixin and see what's already been created there. https://github.com/praekelt/molo/blob/develop/molo/core/tests/base.py#L17

@nathanbegbie nathanbegbie self-requested a review April 3, 2018 09:32
Copy link
Contributor

@nathanbegbie nathanbegbie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much better, well done.

@sewagodimo sewagodimo merged commit 95b53f7 into develop Apr 3, 2018
@sewagodimo sewagodimo deleted the feature/issue-848-polls-via-survey-error branch April 3, 2018 09:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants