Clear interventions after they've been displayed

[eatyourgreens]

Staging branch URL: https://pr-5341.pfe-preview.zooniverse.org

Fixes #5337.

Adds a clearIntervention action to the classifier and calls it from the Intervention component on component cleanup. Changes the nextSubject action so that it doesn't clear any stored interventions by default.

Required Manual Testing

• Does the non-logged in home page render correctly?
• Does the logged in home page render correctly?
• Does the projects page render correctly?
• Can you load project home pages?
• Can you load the classification page?
• Can you submit a classification?
• Does talk load correctly?
• Can you post a talk comment?

Review Checklist

• Does it work in all major browsers: Firefox, Chrome, Edge, Safari?
• Does it work on mobile?
• Can you rm -rf node_modules/ && npm install and app works as expected?
• If the component is in coffeescript, is it converted to ES6? Is it free of eslint errors? Is the conversion its own commit?
• Are the tests passing locally and on Travis?

Optional

• Have you replaced any ChangeListener or PromiseRenderer components with code that updates component state?
• If changes are made to the classifier, does the dev classifier still work?
• Have you resized and compressed any image you've added?
• Have you added in flow type annotations?
• Have you followed the Springer guidelines for commit messages?

[coveralls]

Coverage increased (+0.02%) to 48.9% when pulling 3d7f3ed on clear-interventions into 33eb94a on master.

[camallen]

LGTM minor questions only. Side note is that effects look very nice compared to the existing lifecycle hooks.

[camallen]

My reading suggests this will only run on unmount but it looks like effects will cleanup up if props change as well - https://reactjs.org/docs/hooks-effect.html#explanation-why-effects-run-on-each-update

I don't think this is an issue as we don't pass props to this function, thoughts?

[eatyourgreens]

Good catch. The props here are onUnmount, intervention, user and we probably do want cleanup to run if any of those change.

[eatyourgreens]

This might be useful.

If you want to run an effect and clean it up only once (on mount and unmount), you can pass an empty array ([]) as a second argument. This tells React that your effect doesn’t depend on any values from props or state, so it never needs to re-run. This isn’t handled as a special case — it follows directly from how the dependencies array always works.
https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects

[eatyourgreens]

I've been playing around with this a bit and I think the only time it might happen is if the intervention prop changes while the component is being displayed. Is there a case where I might get a new intervention message while I'm already reading one?

[camallen]

Yes - though in practice it would be slim. They don't send many messages but I could receive one while i'm viewing one, especially if the network / gateway has issues, i.e. received message at gateway, send to sugar but timeout to client who retries.

Would the current code unmount and then update with the newly received message? Should we drop incoming messages until there are no interventions in the store?

I'd say no to switching the store to a list of messages, unless the incoming message has a validity period attached to it, zooniverse/interventions-gateway#22

[eatyourgreens]

My reading of the code is the classifier would receive a new intervention prop, which it would pass down to the Intervention component, which would then run useEffect() because of the props change? I'm not sure what would happen then: maybe the new intervention would render briefly, then be cleared by the cleanup code.

[camallen]

I'm happy to add the validity period to the incoming messages via the gateway API if it helps simplify this issue.

[eatyourgreens]

We could use the second argument to useEffect to ignore changes in the intervention prop. I'm not sure what the implications of that are, but it sounds like a case that shouldn't occur anyway.

At the moment, if a second message were received while you're currently reading one, it would overwrite it and the screen would update to show the new message.