glizzan / concord

Other
1 stars 1 forks source link

Rethink templates #64

Closed shaunagm closed 4 years ago

shaunagm commented 4 years ago

Edit: leaning towards the course of action described in "Simplify the Template System Entirely"

It pains me to say this, since I have spent an absurd amount of time getting our current templating system working, but I think it may still need major changes. Previous thoughts can be found here and here.

Here are some options:

Make "Apply Template" a simple client method and not a State Change

The original version of templates meant that the group had to proactively give everyone "Apply Template" rights just to apply a template, but it didn't even matter that much because to apply a template the actor still has to pass permissions checks for every action inside the template. I hacked around that by changing the apply_template method call to skip the permissions pipeline, but it's still confusing.

If we changed this into a client call and not a state change, we'd need to:

Simplify the Template System Entirely

An alternative would be to remove the ActionContainer logic and just use the "apply template" permission as the main indicator of whether someone can apply the template or not. The problem with this is it's a workaround for people to do things they're not allowed to do. We could limit the damage by checking for foundational changes in the action, and always kick a template with foundational changes to be decided through the foundational decision-making process, but I don't really love this. OTOH it would be way, way simpler.

You know what honestly we should do this. And conditions would just generate automatically because they're done by the system. And save the current setup in a branch to be revisited later.

If we do this, we should be able to remove: ActionContainer, TemplateContext, and a bunch of methods from Template object; and move the bare bones of some of the stuff from Template and Template Context (like replace_fields) into a util called by condition and template model. Could also delete the references to containers from action and condition models, and get rid of some of the methods on the condition client (I think).

Some Other Simplification

There must be a better way to do what ActionContainer currently does, regardless. There's several points of unnecessary complexity, I think. Just not sure how to fix it.

Misc Other Issues

shaunagm commented 4 years ago

Steps

shaunagm commented 4 years ago

Fixed by #94