DH-IT-Portal-Development / ethics

Ethical Committee web application in Django
http://fetc.hum.uu.nl
MIT License
2 stars 1 forks source link

Fix/pre proposals permission denied (571) #577

Closed miggol closed 11 months ago

miggol commented 1 year ago

(edited description) Now ready for review!

What's up?

Acknowledgement: There is so much more to test. I've chosen to draw the line at a basic canary for these specific views. Extending the coverage should be a lot easier in the future.

Weirdness:

miggol commented 11 months ago

Hi @EdoStorm96, because I added some more code I'm re-requesting a review from you. I'm only looking for feedback on the new load_fixtures management command, and loading the test proposals from fixtures.

I do have some questions. Is it necessary for both the BaseProposalTestCase and the MiscProposalTestCase to coexist? It seems like they are largely doing the same thing and could possibly be fused into one, to work for all tests. I think that would be a bit cleaner, but I don't know how much work that would be. I think they could already get cleaned up with some clever inheritance ... I am not opposed to the current state, but maybe you can explain your reasoning having both of these?

The reasoning is quite simple, I do not want to rewrite the entire other ProposalTestcase to use my new fixture. To answer your question, that would be a lot of work. Rewriting tests can be a huge timesink, for no benefit in this case. The other tests still work, so they may remain until they are replaced by something better.