Split the fire firer - Githubissues

Something that has come to my attention via #66 and particularly the https://github.com/WordPoints/hooks-api/issues/98#issuecomment-183764400 offshoot, is that the fire firer currently handles multiple types of events: toggle (or non-repeatable) events, and repeatable events.

The reverse firer, on the other hand, handles only one type of event, since only toggle events specify actions that trigger the reverse firer.

While I'm unconvinced that this should change, I do think that it is something that we should explore, and I wanted to at least document it with its own issue. So here it is.

Theoretical Proposal

The theoretical proposal here is that we would split the fire firer into two different firers. Let's call them toggle and fire for the sake of simplicity.

The fire firer would be used only by the repeatable events, and the toggle firer would be used by the toggle events, to fire the toggle on actions. The reverse firer would continue to handle the toggle off actions of the repeatable events (though I suppose really that could change to, but let's just keep it simple).

The question is, what do we actually gain by this? What are the pros and cons? Why is it such a big deal to have what is essentially a multi-purpose firer?

Background

Let's start with the main issue that we've actually had (or at least thought we had) with the current arrangement.

https://github.com/WordPoints/hooks-api/issues/66#issuecomment-187420112 gives some background for this, and it has to do with how we want the points reactor to handle the toggle events.

When a toggle event fires, we want to award points. When it is reversed, we want to subtract the points (at least that's currently what we're doing). This will never result in a net loss, because a reverse can only happen to reverse a hit that has previously occurred. So we can toggle back and forth between the two states, having the points and not having them, but we can never have multiple reverses in a row, and loose points, or have multiple hits in a row, and gain extra points.

All seems to be well and good, until we start allowing reverses to miss, as we now do. This means that when the toggle off is supposed to occur, there's no guarantee that the points will actually be removed, and so it becomes possible for multiple points awards to build up.

Depending upon the exact situation, there may be times when this is desirable behavior. However, for the points reactor, and in this particular case, this isn't what we want to have happen. The particular case at hand is when we want to disable reversals for an imported points hook that uses that legacy setting. Such hooks are expected to hit only once, not to reverse, and not to hit again.

As a result, we find ourselves wanting the fire firer to behave differently than it currently does. Rather than firing every reaction when it is triggered, we'd want it to behave much more like the reverse fire, where what reactions are fired at is determined based on the hit logs. We'd basically like to fire for each reaction only if the prior hit had actually been reversed, and to skip it if the reverse was a miss. Just like we only fire the reverse firer at a reaction if that reaction has been hit.

Of course, we hesitate to modify the firer to behave this way, because it is currently very generic, while this would be special handling specifically for the toggle events.

That objection disappears if we split the firer. But before we would do that, we need to consider whether this handling would still be too special even for a toggle firer. Is it really generic to how we always want to handle toggle events, or is it specific to this reactor or this use-case? After all, we said above that the current behavior might actually be desirable in some cases.

The question thus comes down to how events and firers should relate, and what the nature of the toggle events is intended to be. This was considered at length in https://github.com/WordPoints/hooks-api/issues/98#issuecomment-183764400, and the conclusion was that the firers which an event chooses to register its actions for are up to the event, and they determine how that event should behave. Thus an event decides how it should behave, and then hooks its actions to the appropriate firers.

So what we really have to decide here is how the toggle events should behave, and then make sure their actions are hooked to firers that handle them appropriately. But then, how should the toggle events behave?

How should toggle events behave?

I suppose the best place to start on this is to consider how the actions that trigger toggle events behave.

https://github.com/WordPoints/hooks-api/issues/98#issuecomment-183764400 explains this pretty well:

The toggle events are events that occur when the status of an entity is toggled. The event cannot have an intermediate state, it either has one or another. Actually, there can be many different status[es], but it can only have one at a time. We might call these quantum events because they can only occur in discrete quantum leap-like changes occur in an entity.

Anyway, the point is that the events flip back and forth between an "on" state and one or more "off" states. We don't really worry about which "off" state the entity is in, we just define the reverse as the transition away from the "on" state to something else (anything else, really).

So a toggle event fires, then reverse fires. That's just the nature of how these actions will naturally occur, not anything that we impose upon them, other than specifying the actions and which ones trigger the toggle on and which ones trigger the toggle off.

This is about fires remember, not specifically hits or misses. And so the proposed special handling in the hypothetical toggle reactor based solely on hits vs misses is just that—special handling.

But this then calls into question the reverse firer, which, as noted above, also includes special handling for hits vs misses.

Reverse firer special handling?

So is the reverse firer's logic special handing, and if not, why is it different than the proposed handling for the toggle firer?

First let's clarify exactly what we do in the reverse firer that involves differentiating misses and hits. When an action triggers the reverse firer, we don't fire for all reactions. Instead we run a query on the hit logs to find matching hits from the fire firer, and then fire only those reactions that the hits were for. Obviously, the hit logs table contains only hits and not misses, so this is where we are differentiating misses from hits.

The reason that we do this is because of https://github.com/WordPoints/hooks-api/issues/68#issuecomment-168016674:

The main point is, that by definition, our reversals should only occur when a prior hit is being reversed. Note that this is about a prior hit being reversed, not just any fire. There is no reason for us to notify the reactor of a reverse when no hit actually occurred on prior fires, because it won't even have been notified that those fires occurred at all.

That last point makes a lot of sense, and brings us back to a concept that has been floating around from https://github.com/WordPoints/hooks-api/issues/104#issuecomment-187940983: secondary firers (or actually, there it was from the perspective of the reactor having a secondary response). The reverse firer is entirely dependent on the fire firer for its operation. If the fire firer never fires, the reverse firer would never fire either. This is kind of obvious when you think about it, but we haven't really considered the implications very closely.

But the question is whether such secondary firers should really exist. Although it makes sense not to fire a reverse hit when no hit had occurred for the points reactor, does this mean that it would never make sense for any reactor?

Actually, based on the concept that only the events should determine firer behavior, is that even a valid question? We keep trying to define our firers based off reactor responses instead. But really that should be irrelevant. It seems though, that reactors do need to have some way of controlling how a firer triggers their responses. We've tried placing this handling in the firers themselves, but now we're moving more toward extensions. Of course, the reactor could always include its own special handling for determining when and how to respond to a hit. It is, after all, supposed to control how the reaction to the event occurs.

So then, if the reason for the current behavior of the reverse firer is purely based on the needs of particular reactors, the special handling should be moved elsewhere. The question is, is it?

As we said above, the nature of toggle events is that they toggle on and off. And this is about fires, not hits or misses specifically. But it does leave open the question of whether an off toggle should be allowed to occur without an on toggle first. In other words, should a reverse fire be allowed to take place with no regular fire before it? It seems quite acceptable that we define reverses this way, and it seems logical that a "reverse" shouldn't occur unless something has actually happened that is being reversed (though of course we could call these toggle_off actions or something else instead of reverse).

Note though that this isn't the same thing as what the reverse firer currently does, which is based on hits, not fires generally. In fact, I think this was the original intention of #68, since it was originally titled "Ignore reverses before fires" (not "before hits"). We decided to do it based on hits because that was what the points reactor needed, and because we thought that (https://github.com/WordPoints/hooks-api/issues/68#issuecomment-168016674):

The design of the API is such that the reverse fire is intended to take place only after a fire has actually occurred (so for the spam fire).

Right now we aren't arguing this though, it is whether this should be true for hits specifically.

If we're specifically listening for the reverses, what we really want is an inverted event.

Perhaps this is simply an indicator that the events API should be different, so that these toggle events only need to be registered once to work both ways. Possibly related to #95.

The reverse and spam fires don't tie the event to particular reactions, so it isn't possible to build a reactor that just works inversely.

This is no longer true.

We can't award points for the reverse fire and subtract them on the regular fire, because we wouldn't know how many points to award. We'd have to pull up the reactions and simulate the handling that happens in the regular fire object within the reactor.

Again, we currently have the reaction settings within the reverse fire, so this is no longer the case.

Also, the event object would have the wrong title and description. So what is needed is really a different event.

This is true, as far as the UI goes. But as I said above, that could really be a problem with the events API rather than with the firer API.

[…] The main point is, that by definition, our reversals should only occur when a prior hit is being reversed.

But where did the definition come from? We can see that to some degree it came from the events API, as that was referenced just before. But it was also influenced by the then-current behavior of the reverse firer (circular reasoning, anyone?), and by the behavior of the points reactor, since both of these were also referenced. In other words, it had to work that way or else the points reactor wouldn't work properly.

Note that this is about a prior hit being reversed, not just any fire. There is no reason for us to notify the reactor of a reverse when no hit actually occurred on prior fires, because it won't even have been notified that those fires occurred at all.

And this is the heart of the issue. We were basing this on the needs of the points reactor, and since it wants to perform reversals only subject to hits of the fire firer, the reversal firer necessarily became secondary to the fire firer, since it was the only piece then in play.

The question we are now trying to answer is not whether the points reactor should work this way, or indeed whether any reactor should work differently, so much as this: should it be the firer logic that supplies this behavior, or not? Let's put all parts of the API in play, and decide where this logic goes.

Perhaps it would be better if the reverse firer were co-equal with the fire reactor rather than being entirely secondary to it.

But then, why should we change this? What do we gain?

From the above we gain several insights:

The reverse firer is secondary to the fire firer.
This was originally done because of the expectation of the points reactor.
We also thought that no reactor could possibly want anything different.
But now we've moved toward defining firers based entirely on the events/actions.
So what reactors would or would not expect should be irrelevant.
Reactors do need to be able to control how and when they respond, obviously.
But now this logic is being placed more in the extensions, not the firers.
We could also place this logic in the reactors themselves.

Thus, it would seem that the logic should be removed from the reverse firer, if:

We accept the current notion of defining a firer purely by the events/actions.
The logic is based on the reactors and not a natural consequence of the events/actions.

However, it is also important to keep in mind that there are really two different bits of logic here. Viz:

Toggle events toggle on and off.
On each toggle each direction a fire occurs.
The reverse firer currently only fires based on hits.
The reverse actions fire based on fires.

So whether the firer should fire only in response to prior fires is different than whether it should keep the logic which it has now, which is to fire only based on prior hits.

In addition question of whether an off toggle should be allowed to occur before an on toggle.

Should events alone determine firer behavior?

There is a big "if", above, and that is our assumption that firers should be defined purely based on the events/actions. Before we proceed, we'd do well to prove that assumption.

What we're really getting down to here is how the API should work. And when deciding that it would be useful to keep in mind another question that was asked above:

Perhaps it would be better if the reverse firer were co-equal with the fire reactor rather than being entirely secondary to it.

But then, why should we change this? What do we gain?

What is the best thing to do here? What are the pros and cons of having the logic in each part of the API?

First let's consider the way that we currently have things working. The logic is in the firers, but can also be spread out over the extensions and reactors. But let's focus on the part of the logic that goes in the firer (thinking specifically of the reverse firer).

The benefit of this is that it is very concrete and well defined. It is easy to grasp with the mind because a firer does something pretty specific. Putting the logic in an extension would work, sure, and we're doing that some. But when it becomes more composible like that, it also seems more tenuous and fragile within one's mind, so to some degree it is harder to follow. The logic isn't tied to the reactor in some concrete way (as when it implements the interface necessary to listen for a particular firer), so it can be difficult to keep it all straight in one's head. Of course, placing the logic in the reactor itself would solve this problem, since it would be very obvious then. The issue with that, though, is that then the logic isn't re-usable anymore. If two reactors want to behave the same way, they'd have to duplicate the logic, which is less than ideal.

Which is ultimately why we were putting this in firers. However, we've also realized another issue here. And that is that the concrete nature of firers can be just as much of a hindrance as a help. It is easier to follow, but it is also inflexible. The events define what actions should trigger what firers. And to some degree, this is actually a problem in the API, although it is how we've been coming to define firers. Because the event is then determining, more or less, what type of responses a particular action can have. This is by design right now, but what happens when the firer it specifies is too specific in how it responds to those actions? What happens when later another reactor wants to behave differently, and it can't use that fire to listen to the event, because it wants to behave differently than that firer will allow? In a case like that there is really no easy way forward, because to introduce a new firer (which is basically a necessity at that point), would require that each event you want to listen to have those actions hooked to that firer. The only way that you'd be able to achieve that would be to loop through the events and take all of the actions that are hooked to the old firer and hook them to the new firer you want to listen to as well.

But again, that's not ideal. It would be better that we don't put ourselves in that place to start with. As I see it, there are two ways of avoiding it:

Don't allow any special handling in the firers, so that they are as generic as possible.
Modify the API so that multiple firers can listen to the same action type.

It's possible that a combination of these would be even better.

Generic firers

First let's consider the idea of making the firers as generic as possible. What this basically means is that the firer itself should not contain any logic that restricts an action from being fired at every listening reaction. It would seem then that basically all of the firers would merge into a single action-agnostic firer. We wouldn't need the firer registry anymore at that point, so we'd likely just move the firer logic into the router.

There are a couple of issues there though:

What about #70, which says that we should fire reverses even after a reaction has been deleted? The generic firer logic would only fire at each existing reaction, so it wouldn't do that.
How would reactors listen only for specific action types (with the per-firer interfaces no longer needed)? We're basically implying that reactors are firer-agnostic at that point (see #104), or else we'd have to let each reactor specify a list of action types in a property or something.

The special handling logic would have to be moved to the reactors, or to the extensions (DRYer and more composible). Moving it to the reactors has the problem that once your inside the reactor, the fire has already be logged as a hit. If it really isn't a hit, that's likely going to cause issues (or at the very least is wasteful and bad API structure). So although the reactors can compose things in various ways, the logic itself would need to be placed in extensions.

The main issue with extensions, noted above, is that they seem to detached from the reactors themselves, thus making it more difficult for the mind to grasp the true nature of the reactors' intended behavior. Of course, this is not necessarily a problem with the API, and may be more an issue with our perception of it. Still, I should note that it would be possible, if we so desired, to allow extensions to be more tightly coupled to reactors in some way.

Extensions do raise a serious question though, in that they block hits, not the fires themselves. When the logic is placed in the firers, it can block a fire from ever occurring at all. It isn't a miss, the trigger was never pulled. The extensions on the other hand don't have that control. By the time we're in the extensions, the trigger has been pulled, and it is their job just to determine whether the fire was a hit or a miss.

Are there substantial consequences of this?

The current logic, presently contained in the reverse firer, does include handling for the misses as well as the hits. (It saves meta data for each hit that was being reversed, reversed_by => 0.) So were fires to occur less discriminately, we might think that the extra fires, though blocked from hitting, would still receive undue handling as misses. This would be a problem if it were so. However, the logic would then be contained in an extension entirely, since we would have one generic firer which would contain no such special handling. And since the extension would have full control over the logic, it could work around this issue by only acting on those misses that it should act upon—those not blocked by itself but by later extensions. (Related to that is #17 which deals with a similar issue for the rate extension. Again, the extra misses will not cause problems, but the rate extension will need to know that the misses weren't caused by itself, so that it doesn't count them toward the rate quota.)

But though the particular logic in question does not raise an issue in and of itself, it does bring to our attention the fact that we'd have to now reintroduce the ability for extensions to listen for misses, or else such an extension as is needed could not exist.

The only alternative would be to change the logic in some way to work around this if possible. I suppose actually that this would be possible, because we could mark all of those past hits as misses in the should_hit() method, and then mark those that hit as such only later in the after_hit() method. This is only a hack, of course, and it would probably lead to unintended side-effects for any other extensions or reactors that queried the hit logs in some way. In fact, this problem already exists with the current arrangement, especially if extensions run queries that span across reactions, because the metadata is only updated by the reverse firer after all of the reactions are looped through.

Anyway, if we go this route, we'd need to implement an after_miss() method for extensions, in addition to finding solutions to the two bullet points above.

Multiple firers per action

The alternative, noted above, is to allow multiple firers to listen to the same action type. So for example, we'd have the fire action type, and we could have a fire firer and toggle_on firer both listening for this type of action. This would likely be achieved simply by making the firer registry a children registry, with the action types as the parents for each set of firers. This would make it easy for the router to grab a list of all of the firers to trigger, and then it could loop through them.

Which reactors listen to which firers could be determined in the same way that it is now, so we wouldn't have to invent a new piece of the API, like we would have to do if we did the above.

We also wouldn't have the issue regarding #70 and listening for deleted reactions if we wanted to, because each firer would be free to do that, just as they are now.

And we also avoid the potential for issues regarding misses vs fires that were never triggered at all, since each firer can continue to choose to fire at only certain reactions rather than all of them.

The implication though, is that events no longer have any determination on how the firer works. The firers can basically do whatever they want, and have whatever kind of logic they want. And I think this could be a detriment to the API because it could fuzz the line between firers and extensions. It would become a question of what should a firer vs an extension do, rather than what can a firer vs an extension do, which makes the API much more defined.

So it would sort of call the purpose of firers into question.

Also note that because the events don't determine the behavior anymore, this could cause strangeness in the API, since it isn't which event a reactor is listening to, but how it is listening to it. We assume that reactors would listen to a strict set of firers, and would do so consistently, but it just seems like the event name and description being used in the UI might not really convey the way things would behave accurately all the time anymore. It is true that we can modify behavior right now using extensions, and that this doesn't have to be done transparently, but the event no longer knows exactly what it is or how it occurs, in the way that it could now. The event and the firer are then completely decoupled. (OK, they're decoupled now, but the event at least knows how the firer behaves by default.) But I suppose in a truly composible API, there should be no behavioral expectations for any other part of the API. The main problem with that is that it makes it difficult to build a UI with i18n in mind, because currently we let the events title and describe themselves. But it becomes harder for them to do that if they don't know how they will behave. I guess in that sense they presently act as umbrellas for a particular behavioral unit of the API, not just one composible component of the API. In other words, events are composers.

Having composers isn't a bad thing. In fact it is necessary for composition to take place at some level of the API. The question is whether this should happen at all in the API itself, or only in the UI and reactions. But that is sort of another question for another time. And note that events would still be composing the actions themselves anyway.

The truth I guess is that this is an issue we would have with a uni-firer too anyway, because then this logic would be provided by extensions that the UI/reaction/reactor would have to control, and the behavior would no longer be left up to the event to determine. But it just seems different somehow. Maybe because the events would continue to play a role in setting the default standard for behavior from a particular action type, since there would be a single, concrete firer whose logic wouldn't have any surprises. Sure, the logic that could have been in a firer will still be there in an extension, but it seems different. I guess because the events themselves could no longer imply that particular unit of behavior. Sure, anything can be added on via extensions, but the event itself would be entirely decoupled from that and couldn't be counting on it in any way.

I suppose with action types being decoupled from particular firers we have the same thing, but in that case the events have absolutely no idea what a fire(s) would take place, while with the uni-firer they at least have a constant baseline, and any variation in that baseline from one event to the next comes from the actions.

So I guess one question we have to answer here is which way we want this to be. And I honestly am thinking that it makes more sense for the events to have a single job of composing the actions, and not composing the firers (which both options do), and for them to have a baseline to go off of so that they can describe themselves (which only the uni-firer offers).

Without the baseline, I think we can honestly ask why the events should exist at all. Since they can't really describe themselves meaningfully, they cease to really have any purpose. Why can't the firer just hook directly to the actions? But then reactions listen to particular events, not to firers so that wouldn't work (unless we made the reactions specify which actions and firers to listen to). So I guess the events are necessary to compose the actions into meaningful, coherent groups. But then if that is so, it just confirms my point above that they are useless if they can't coherently describe what they are because of the unknown of the firers.

Unless of course, maybe we're just trying to describe events in a different way than we should. How should an even describe itself? Should it describe all of the different types of actions that it composes? But then it is possible for more actions and action types to be added to an event anyway. But I suppose we could just consider that an edge-case.

Or, perhaps this should really just be left up the actions. Then we'd compose a description from the firer description (something we'd have to add) and the action description, like this:

 {firer 1 fires when}: 
- {action 1 description} 
- {action 2 description}

{firer 2 fires when}:
- {action 3 description}

So like:

Occurs when:

a post is published.

Reverses when:

a post is depublished.

a post is deleted.

Then the event itself would have no description at all, just a title, I guess.

This is actually an appealing prospect, and I think it might make more sense then what we do with event descriptions now. But I'm sure there are more questions to be answered, like what about dynamic actions. And really most actions don't have particular classes at all, so this will increase the number of classes necessary unless we do translation up-front, which I'm loath to do.

I guess we could even go so far as to ask whether we should even be composing the action within the API, or whether this is better left up to the user (which would mean the reactor would compose the actions, as suggested above). But then that doesn't really make sense, because this really has to be done with care or else the event args won't match up. And it is strange to put this on the user anyway, it would raise the entry bar considerably.

I think that the issue regarding #70 could be handled with reactor chaining. However, I'm really unsure that it makes sense, or that it makes sense within the context of the API itself, rather. Especially now that extensions are allowed to abort reverse firers, it doesn't make sense to continue to run reverses for deleted reactions. Instead, what should happen is that the reaction should be partly deactivated/disabled in some way if part of its operation is to continue. Exactly what that looks like in the UI is up to a particular implementation to decide.

Double Reverses

Something that I happened to think of, with the unified firer, what happens a post is depublished an then deleted. Will the event reverse action fire twice?

Having tested that, I've found that the answer is yes. Is this expected behavior?

Perhaps we could have a combo uni-firer/multi-firer, where instead of multiple firers the event was allowed to modulate the cadence, or perhaps an action type object could determine whether the fire should occur at all. There would only be one of these objects per action type, of course. And it would be blocking a fire from ever occurring, not blocking hits. It would differ from the way that firers do this because it would block the fire entirely, not on a per-reaction basis. But then how can it really do that without any miss logs, only hit logs? It can't tell whether the action type has fired twice, which I guess is also true for an extension or anywhere that we would put this logic. So this idea really wouldn't work.

The solution to the question regarding post depublish vs delete above then, is to introduce a special action object for the delete action that only fires if the post hasn't been depublished. I'll open a new issue for that.

Also, if we keep the current logic (not still in the reverse firer, but somewhere) we keep track of the reversed_by meta key and set it to 0 when there is a miss. And in that case we don't have to worry about this anyway, because it allows us to only fire for those reactions that haven't been reverse fired already (whether hit or missed), which is the concern there. (Though I still think we should consider fixing this at the action level.)

So we'll likely want this logic somewhere, but to some degree it is a per-reactor thing. It would be reusable if we moved it to an extension rather than a reaction though. But if we move it to an extension, that extension wouldn't work properly if it was added after the reaction was created, would it? because so many hits/fires of that reaction would have already occurred that wouldn't have been marked as reversed. So the meta key wouldn't be set for them.

I guess that maybe this is why it makes more sense for this to be a part of the reactor logic (or hard-coded into the reactor that it saves these settings for each reaction). Or, perhaps some reactors can just decide to let it be implicit in the way that they handle the reversals (as the points reactor sort of does in that it checks only for unreversed transactions in the logs query).

This is all related to #99.

It's worth keeping in mind here that there are two basic pieces of logic that we are dealing with in the reverse firer:

The basic query, which pulls only reaction IDs for hits that have occurred, and
The specific meta key query and handling, which is about reversing only hits from the last fire.

The points reactor needs both of these things to be true, though we're still unsure exactly where the logic should be, in the reactor or in extensions. Possibly a mix of the two.

Unified Firer

I've gone ahead and done the unified firer approach. The only real downsides to this that I noted as I was going along were that it means that we now pull up every single reaction when we might not really have to (previously we avoided that in the reverse firer), and that we can't modify firer behavior as easily (like for #17). As far as that latter point goes though, we can still override the whole router, which I guess would be correct now since we want to change the logic for all action types.

The main unsolved issue then would be how to handle the event descriptions and titles in the UI, which I think we provided a solution to above. That solution outlined there would work for the uni-firer approach just as well as for the event-decoupled firers. The only difference would be that in this case there are no firers, and so the description for each action type wouldn't be supplied by the firer object. But far from being a downside, this actually gives greater granularity, since now this can be supplied by the reactor (or just built into the UI, etc.). (Remember that reactors have to specify the slug of each action type they wish to listen to, so we aren't asking them to provide a description for an action type that they might not even know about.)

Of course, we wouldn't actually have to make those changes, but it is one of the questions surrounding #68.

Splitting the Action Types

Which finally brings us back to what this ticket was originally about to start with. The fire action type is used by multiple different types of events. The only difference is that now we're talking about splitting action types instead of firers, but its the same thing.

So we may still want to split fire action type in fire (or perhaps something else) and toggle_on.

But before we just jump in and do that, let's examine again why we want to do this.

The main gist of it is that we have two different types of events: repeatable and toggle. These event types aren't really prescriptive, but rather descriptive of the way that these user actions work. Action types are also mainly descriptive, as they describe how that particular action naturally relates to that particular event.

The question is, why is it a problem that multiple event types use the same action type? In fact, event types aren't really a part of the API at all. But perhaps they should be, see #59. I guess that is a discussion that belongs in another ticket though, even if we think that our decision here depends upon it.

I guess its pretty simple. Action types are supposed to be descriptive, so they need to provide a precise description, otherwise they aren't really worthwhile. The question then, is whether action types are global, or specific to particular types of events? Currently we treat them like they are event specific, so that fire can mean something different for toggle events than for repeatable events. One reason that we did this was because we wanted to respond the same way to both of them in the points reactor (and because they've always been that way in the points hooks API). However, it is now even easier than it was before to handle two action types in the exact same way, since they are all currently routed through the reactor's hit() method.

So we are free to make the action types global across different event types. And I think that that might make the most sense, because then a reactor could handle all actions of a certain type the same way regardless of event type. But I'm not sure that that is even useful, because different types of events probably wouldn't ever really need the same action type, would they? After all, that's sort of why we're contemplating splitting these in the first place, because they mean different things.

I think it is worth considering what we said above:

Action types are also mainly descriptive, as they describe how that particular action naturally relates to that particular event.

So can an action relate to two different kinds of event the same way? Maybe yes, maybe no. Actually, I think it doesn't really matter. The point is that these do not. So we're better off splitting them, and then we can answer that question as we go along. At least that way it is less confusing, rather than having a single action type that can behave in wildly different ways.

The main question then, is what to call each of the action types we spit this into. Maybe we should just go with toggle_on for the toggle events, and maybe change reverse to toggle_off. And then we can always worry about changing these to something better later, as we start to get more familiar with the new feel of the API.

One side-effect of using the same names for action types from different events was that it allowed us to reference a single response type from the reactors across different types of events. The toggle events and repeatable events both wanted the same response from the points reactor, for example, so they both called that action type by a single name.

With split action types however, we can no longer do that. This becomes an issue if and when we introduce a new event type, because it will have action types that no previously-created reactors would be able to listen for. Before we could just call its action type the same thing as one of the existing action types, and reactors would automatically give that same response. But now we can't do that.

(Note also that this means that now every reactor can't listen for just any event, which is something to consider when displaying the events in the UI. Actually, I guess this was true before, but we didn't really think of it. Anyway, I guess that is for its own ticket.)

So it seems that we need to make the reactors and action types more composible. Instead of the reactor composing them, perhaps there should be a registry or something (maybe on the router like with the actions?) and they should be hooked up externally. That would allow for the reactors to be configured for types of events that they didn't know about, and also give full flexibility in configuring how those reactors should respond. So we could match up the fire event with the reverse response, and vice versa, if we wanted to.

That is assuming, of course, that we can also compose the reactor responses, which isn't currently the case. So I guess that would also have to be composed externally, rather than internal to each reactor as it currently is. As things stand, each reactor has to check for different action types internally and route them to different responses if that's what it wants to do. But I guess it makes more sense for that to be in the router, doesn't it?

Even now that we're thinking of moving the composing of responses out of the reactors though, we're still just assuming that this should be a per-reactor thing. But maybe there is a case to be made that each reaction should be allowed to control this if it wants to. After all, reactions already have to know about the event they are attached to, so they have to know about the action types that accompany it (or at least those that they want to react to). So it makes sense that reactions should be able to tell the router how to route action types to reactor responses.

I've actually been thinking about how reactions listen to events, and not action types (or firers, previously), but that perhaps that should change. And I think it is starting to make a lot of sense. I think it would also make sense to revisit some things that we discussed #104, like splitting the reactors so that the each have just one response, though there is still a lot of things to work out there.

I guess now that we've split these though, we're really done here and this discussion should be continued in new tickets.

One more thing here, in the UI the action types aren't determined correctly. They are based on info from the reactor, but they should be based on info from each event. Related to #110, which I guess fixing this might be partly dependent on.

I've opened #130 to address that last issue, so we can close this.

WordPoints / hooks-api

Split the fire firer #108