podemos-info / decidim-module-crowdfundings

GNU Affero General Public License v3.0
3 stars 1 forks source link

Simplify query specs #49

Closed deivid-rodriguez closed 6 years ago

deivid-rodriguez commented 6 years ago

:tophat: What? Why?

Simplify query specs so that they don't create that many objects.

:pushpin: Related Issues

None.

:clipboard: Subtasks

None.

:camera: Screenshots (optional)

None.

:ghost: GIF

None.

deivid-rodriguez commented 6 years ago

One record of each kind and asserting the exact expected result seems enough for me, but I haven't really looked at the implementation. I can have a look to see if some coverage is being lost here. I'll also double check your second bullet point since I'm there.

deivid-rodriguez commented 6 years ago

So I looked at the implementation and I think the new tests are good to cover the functionality. Previous version was overkill. They run all in ~5s including boot on my machine, so I'd say we're more or less fine there.

Regarding your other comment, I think the implementation is ok, at least those called Pending*Collaborations. They are used for offline tasks to be launched periodically to renew recurrent collaborations, so I'd say it's expected that they loop through all user collaborations (regardless of their organization). The naming is once more terrible, though. The organizations we want are not the ones with pending state, but the ones with accepted state, so Renewable*Collaborations would've been a much better name.

deivid-rodriguez commented 6 years ago

I'll reticket the renaming as a separate issue, if that's ok @leio10. The naming in this project is very hard to get right :sweat_smile:.

deivid-rodriguez commented 6 years ago

:tada: