mattjegan / django-gamification

The missing Django Gamification Package
Other
166 stars 28 forks source link

Implement chained badges #11

Closed gitsteph closed 6 years ago

gitsteph commented 7 years ago

Hi! I'm not sure what the status was on this feature; seemed like it was partially implemented then commented out for some reason... submitting this since there weren't updates in the issue on it yet. :)

A few things I noticed/questions:

Finally, thanks for maintaining this!!

(Related to Issue #3 )

mattjegan commented 7 years ago

Thanks for making the PR into the project, I'm always looking for help. Some great points you've mentioned above 😄 You're first point (interfaces not creating missing badges) is definitely a bug which we'll need to address at some point. As for the second point, I did originally intend for BadgeDefinitions to be used to create Badge instances automatically, do you think this could be improved or perhaps just documented better? Lastly, my example of using Badge.objects.first() was intended to demonstrate the use of Badge.award() rather than fetching of a particular Badge, perhaps this could also be documented better?

mattjegan commented 6 years ago

Thanks, looks correct to me 👍 I'll merge it in right now. One thing that I would perhaps recommend in the future is to split your tests into multiple smaller tests. In test_save, whenever you think you need another test_msg# that would indicate that you may need another test_ method instead 😄 Anyway, thanks for the awesome contribution and I hope that you may continue to contribute to this project.