Implement control mechanism for queue release #225

emanueleDiVizio · 2019-10-01T14:14:52Z

Motivation

While using this library, we encountered the need for halting the queue and resume it based on different conditions. In our app, we have two use cases for this. Since we see other people having the same issue (#160), we thought to write a PR.
First, wait for the user to authenticate for the queue to be released. Second, we treat certain logic as blocking, so the queue will wait for the logic to be complete before resume.
This is implemented with a simple semaphore mechanism.

Test plan

The idea behind this PR is to have a semaphore mechanism for the queue. The logic is very similar to the one used to handle connectivity change.
We have a semaphore variable hasQueueBeenHalted.
To toggle the semaphore we've implemented the queueSemaphoreChange(shouldHaltQueue: boolean) action. If shouldHaltQueue is set to true, then hasQueueBeenHalted will be set to true and the queue will stop being released.
If shouldHaltQueue is set to false, then hasQueueBeenHalted will be set to false and the queue will be resumed. This is all done only if either isConnected is true or it has been recently toggled.

I've added a couple of tests for the functionality.

Note, this is a working MVP at the moment. I'm pushing a PR already to gain early feedback and to let fellow developers benefit from this. Do please comment on this and share how it can be improved.

Closes #160

melvynhills · 2019-10-01T16:28:07Z

Clean implementation 👌

I personally would prefer naming this API in a simpler way, abstracting away the pattern that is used (semaphore), i.e. having actions called e.g. enableQueue() and disableQueue() or even toggleQueue(bool) instead of queueSemaphoreChange(bool).

However, how would you compare this imperative "ON/OFF" API vs. something more declarative like I did here? It allows me to just specify one single state selector that can enable/disable the queue automatically, as it is just a function of the current application state. Typically it works like this:

const networkMiddleware = createNetworkMiddleware({
  shouldDequeueSelector: state => state.session.isAuthenticated,
  ...
})

Anyway, this looks like a nice & useful addition to the library!

emanueleDiVizio · 2019-10-02T07:24:26Z

Hey, thanks for the feedback! I did think about the naming, more than happy to rename. How about stopQueueRelease() and resumeQueueRelease()?

Regarding the API, having a selector in the middleware config was indeed something we thought about, yet we didn't see this implementation. I think it's cleaner, thanks for the tip!

Question, in your code I don't see any further check in createReleaseQueue, which as I understand would serve to halt the queue resume process if it's already running. Did you manage to find a way to do so without modifying createReleaseQueue?

emanueleDiVizio · 2019-10-02T08:51:30Z

Alright, I've implemented it in a cleaner way following @melvynhills suggestion. I've opened a PR on our fork as I'm short on time to fix unit tests. If people want to take a look, here's our implementation with a deselector. https://github.com/evrythng/react-native-offline/pull/2/files. Thanks @melvynhills for the tip!

rgommezz

Nice work, thanks for this! 🎉

In terms of the implementation, I'd suggest a couple of changes:

1.- Move the flag hasQueueBeenHalted into createNetworkMiddleware, inside the closure, instead of a new reducer piece of state. I feel this is something internal to the library, so it fits better if it's not publicly exposed to the consumer. Furthermore, I wouldn't expect that driving any UI state.

Something along these lines:

function createNetworkMiddleware({
  regexActionType = /FETCH.*REQUEST/,
  actionTypes = [],
  queueReleaseThrottle = 50,
}: Arguments = {}) {
  let hasQueueBeenHalted = false; // We keep it in the closure
  return ({ getState }: MiddlewareAPI<State>) => (
    next: (action: any) => void,
  ) => (action: any) => {
    const { isConnected, actionQueue } = getState().network;
    const releaseQueue = createReleaseQueue(
      getState,
      next,
      queueReleaseThrottle,
    );
    ...

Then the new action would just mutate that variable and the queue would operate based upon it.

2.- Rename the action creator as per my comments. Also, maybe change the payload to an enum (object) we could export, something like Semaphore.RED, Semaphore.GREEN, to be more descriptive. Passing a boolean doesn't reflect perfectly the intent. Also, we could extend it in the future for other use cases.

Looking forward to hearing from you :)

src/redux/actionCreators.js

src/redux/actionTypes.js

src/redux/actionCreators.js

melvynhills · 2019-10-03T20:12:54Z

@rgommezz I think @emanueleDiVizio's plan is to do it in a more declarative way, as most people seem to like the idea of a state selector: https://github.com/evrythng/react-native-offline/pull/2/files

rgommezz · 2019-10-03T23:21:43Z

Oh cool @melvynhills, sorry I missed the last comment and the separate fork, went straight to the code.

I like that approach as well and would definitely be up to get this merged, could we make this happen @emanueleDiVizio? I can help with the tests if needed, just point your changes to this repo.

emanueleDiVizio · 2019-10-07T15:04:05Z

So quick update, I've ended up using both mechanisms in our codebase, as I couldn't find a way to stop the queue with the selector if it's already been released.
Code for both approaches being used at same time is here emanueleDiVizio@59244e6.

I couldn't manage to update this PR yet, so code is in my fork for now. If you guys are happy to have both approaches available, I'll be glad to clean up code and update this PR. I'll also address @rgommezz comments 👍

To speed this up, @rgommezz (and everyone else who's up for it) would you mind checking out emanueleDiVizio@59244e6 to see if anything could be improved? I'll then open a PR here asap.

emanueleDiVizio · 2019-10-09T10:57:28Z

Okay great, I've managed to have both approaches side by side.

In the last two commits, I've implemented community suggestions and refactored a little bit. We also have tests for new functionality. Think I've ended up having both community suggestions and state selector into a single commit, apologies. Next thing I think would be to update the docs.

@rgommezz I've had a thought about moving the queue flag out of the reducer, yet I didn't manage to do it just yet. The point is that createReleaseQueue relies on redux state being updated in order to stop the queue, and I'm wondering if updating the variable in the parent closure would be seen by createReleaseQueue straight away as well. Probably yes, yet I'm prioritising not breaking current functionality over refactor, as we're using this in our app.

If anyone else (or me later on) manages to move out the isQueuePaused flag from the reducer, I'll be more than happy.

Let me know if there's any other way I can help!

Edit:

I ended up trying @rgommezz approach, that is using an inner variable instead of using the store.

The problem of this approach is that a new changeQueueSemaphore is only read after the queue loop is finished, hence any change to an inner variable gets read-only after the queue is flushed, thus preventing halting mid-way.
By storing the semaphore variable in the store we work around this problem, as the store is updated before the queue loop goes to the next item. Hence, const { isConnected, isQueuePaused } = getState().network; always returns the correct value.

If anyone has any idea on how to improve this, I'd be more than happy to listen!

emanueleDiVizio · 2019-10-09T14:35:11Z

Update, after last push createNetworkMiddleware test seems to get stuck. I now have very little capacity to work on this, so if anyone else could investigate it'd be very appreciated.

emanueleDiVizio · 2019-10-12T09:00:19Z

Alright, I've fixed it, tests are now passing. Semaphore is working and deselector is working. Only thing is if queue deselctor changes from true to false, the queue is not automatically released. Apart from this, doc update is last thing left I reckon. @rgommezz If everything looks good, I can provide docs 👍

emanueleDiVizio · 2019-10-12T09:08:24Z

Btw @rgommezz, I can't fathom why but I have been using a DAZN pen for the past few days :') I've found it in my office...

MakhouT · 2019-10-12T09:16:06Z

I am happy to test it if you can give me some pointers on how to use it. I am using redux-saga

emanueleDiVizio · 2019-10-14T07:37:59Z

So we have two ways to control the queue. One is with shouldDequeueSelector and the other with semaphore actions. shouldDequeueSelector is a func returning bool. If true, then the queue can be released, if false queue release is prevented. Note, this only works before the queue release is initiated. If shouldDequeueSelectorsuddenly returns false while the queue is getting released then it won't be stopped. If we want to halt the queue while it's being released we have to dispatch aCHANGE_QUEUE_SEMAPHORE action, with 'RED' payload.
This action works in the following way: if semaphore is RED, queue release is halted, if GREEN then it can go. Let me know if more info is needed. No difference with saga, just yield the semaphore action. Thanks for testing this!

MakhouT · 2019-10-14T07:49:39Z

I'll try it out this week if I get some time. When I've tested it, I'll get back to you!

emanueleDiVizio · 2019-10-14T10:22:59Z

Sweet, thank you @MakhouT! @rgommezz is there anything else to do to make this happen?

rgommezz · 2019-10-16T00:13:10Z

@emanueleDiVizio I briefly glanced at it during the weekend and it look quite solid, well done! 🎉 I'll fully re-review tomorrow

rgommezz

Left some comments, but it's almost ready!

Please revert the yarn.lock addition. This repo uses npm

rgommezz · 2019-10-16T00:48:26Z

src/redux/createNetworkMiddleware.js

+function didQueueResume(action, isQueuePaused) {
+  return (
+    action.type === networkActionTypes.CHANGE_QUEUE_SEMAPHORE &&
+    !isQueuePaused &&


Wouldn't this entail that the queue was paused before?

So the condition would be:

return ( action.type === networkActionTypes.CHANGE_QUEUE_SEMAPHORE && isQueuePaused && // shouldn't it be like this? action.payload === 'GREEN' );

I believe we're checking that the queue was paused before this action is received, since by the time the middleware receives CHANGE_QUEUE_SEMAPHORE, isQueuePaused hasn't been updated yet. So I think that !isQueuePaused is the correct check. Does that sound right?

As you are saying, we're checking that the queue was paused before this action is received, so isQueuePaused === true at the moment the action arrives to the middleware. The semaphore action indicates that we should unblock/unpause the queue.

So for me, didQueueResume means that:

And action of type action.type === networkActionTypes.CHANGE_QUEUE_SEMAPHORE is about to be dispatched.

The queue is currently paused: isQueuePaused === true

The payload is SemaphoreColor.GREEN, meaning we should unpause/unblock the queue

Ah, I see! yes good catch, I'll fix this 👍

Can confirm fix is in d0393a6

Alright done, do we also want to export SEMAPHORE_COLOR const?

You changed a different function on d0393a6 😕

True, I've thought about it. Not sure how it plays with Flow tho, since we've defined enums for it... Should I just add a SemaphoreColors object without flow?

Yes, I think that's better. Once we migrate to Typescript in the future, we'll be able to leverage Enums

Uh, my bad! Will fix it 👍

rgommezz · 2019-10-16T00:51:57Z

src/redux/createNetworkMiddleware.js

-    if (isBackOnline) {
+    const hasQueueBeenResumed = didQueueResume(action, isQueuePaused);
+
+    const shouldDequeue =


What about this regrouping, since isBackOnline by itself was already a valid condition?

const shouldDequeue = (isBackOnline || (isConnected && hasQueueBeenResumed)) && shouldDequeueSelector(getState());

Good idea, I'll implement this 👍

rgommezz · 2019-10-16T00:53:32Z

src/redux/createNetworkMiddleware.js

 }: Arguments = {}) {
  return ({ getState }: MiddlewareAPI<State>) => (
    next: (action: any) => void,
  ) => (action: any) => {
-    const { isConnected, actionQueue } = getState().network;
+    const { isConnected, actionQueue, isQueuePaused } = getState().network;


Tiny opt. Let's keep the state in a variable right away since we are calling getState() twice.

So the reason for calling getState inside the releaseQueue loop is that this way the queue release function always has the latest state on every iteration. This way, when CHANGE_QUEUE_SEMAPHORE and CONNECTION_CHANGE are dispatched the new state is reflected straight away inside releaseQueue, since at every iteration we call getState(). On the other hand, if we pass down isQueuePaused and isConnected from middleware function their values will only reflect the state when the releaseQueue was created, and will not update on further state change while the queue is being released. Does that sound right?

rgommezz · 2019-10-16T00:54:21Z

src/types.js

@@ -4,6 +4,8 @@ export type State = {
  isConnected: boolean,
 };

+export type SemaphoreColor = 'RED' | 'GREEN';


emanueleDiVizio · 2019-10-16T12:14:50Z

Done, I've removed the yarn.lock and regrouped the shouldDeque expression. I've left a couple of comments 👍

emanueleDiVizio · 2019-10-16T15:46:49Z

Alright good news, I've also updated docs. Let me know if anything can be improved there!

…ative-offline into feature/queue-semaphore

rgommezz · 2019-10-16T16:41:02Z

Btw @emanueleDiVizio, I can't merge this without having 100% coverage to make sure there are no regressions.

You can take a look by running npm run test:coverage locally.

It seems that function (didQueueResume) I spotted that seemed faulty is not properly tested and that's why the tests didn't catch that.

emanueleDiVizio · 2019-10-17T09:52:39Z

Sure thing, I'll clean up and address tests between today and tomorrow 👍

…ge to 100%

emanueleDiVizio · 2019-10-17T15:17:33Z

Alright good news, I've fixed didComeBackOnline and didQueueResume functions and I've brought back coverage to 100%! I believe we're close, let me know if there's anything else I can do 👍

rgommezz

Nice, thanks for the changes.

I've added some last suggestions for polishing (you can commit them directly from GitHub) and a couple of comments regarding tests.

This is pretty much ready to be shipped 👏

rgommezz · 2019-10-17T23:39:22Z

README.md

@@ -347,6 +347,7 @@ type MiddlewareConfig = {
  regexActionType?: RegExp = /FETCH.*REQUEST/,
  actionTypes?: Array<string> = [],
  queueReleaseThrottle?: number = 50,
+  shouldDequeueSelector: (state: State) => boolean = state => state.conditionToReleaseQueue


Suggested change

shouldDequeueSelector: (state: State) => boolean = state => state.conditionToReleaseQueue

shouldDequeueSelector: (state: RootReduxState) => boolean = () => true

rgommezz · 2019-10-17T23:44:25Z

README.md

@@ -360,6 +361,9 @@ By default it's configured to intercept actions for fetching data following the

 `queueReleaseThrottle`: waiting time in ms between dispatches when flushing the offline queue. Useful to reduce the server pressure when coming back online. Defaults to 50ms.

+`shouldDequeueSelector`: state selector used to control if the queue should be released on connection change. Returning `true` releases the queue, returning `false` prevents queue release. Note, if the result of `shouldDequeueSelector` changes *while* the queue is being released, the queue will not halt. If you want to halt the queue *while* is being released, please see relevant FAQ section.


Suggested change

`shouldDequeueSelector`: state selector used to control if the queue should be released on connection change. Returning `true` releases the queue, returning `false` prevents queue release. Note, if the result of `shouldDequeueSelector` changes *while* the queue is being released, the queue will not halt. If you want to halt the queue *while* is being released, please see relevant FAQ section.

`shouldDequeueSelector`: function that receives the redux application state and returns a boolean. It'll be executed every time an action is dispatched, before it reaches the reducer. This is useful to control if the queue should be released when the connection is regained and there were actions queued up. Returning `true` (the default behaviour) releases the queue, whereas returning `false` prevents queue release. For example, you may wanna perform some authentication checks, prior to releasing the queue. Note, if the result of `shouldDequeueSelector` changes *while* the queue is being released, the queue will not halt. If you want to halt the queue *while* is being released, please see relevant FAQ section.

rgommezz · 2019-10-17T23:50:33Z

src/redux/createReducer.js

@@ -8,11 +8,13 @@ import type {
  FluxActionWithPreviousIntent,
  FluxActionForRemoval,
  NetworkState,
+  SemaphoreColor,
 } from '../types';



Suggested change

import { SEMAPHORE_COLOR } from '../utils/constants';

Have to keep SemaphoreColor for typing in handleChangeQueueSemaphore (line 88). I'll import SEMAPHORE_COLOR as well.

rgommezz · 2019-10-17T23:50:53Z

src/redux/createReducer.js

+): NetworkState {
+  return {
+    ...state,
+    isQueuePaused: semaphoreColor === 'RED',


Suggested change

isQueuePaused: semaphoreColor === 'RED',

isQueuePaused: semaphoreColor === SEMAPHORE_COLOR.RED,

rgommezz · 2019-10-17T23:58:04Z

test/createNetworkMiddleware.test.js

+    expect(mockDispatch).toHaveBeenNthCalledWith(4, 'bar');
+  });
+
+  it('does not empty the queue if we are online and dequeue selector returns false', async () => {


Help me understand this test, if the dequeue selector returns false, the queue is not emptied. However, I can see actions dispatched about removing stuff from the queue.

I think our best shot here is to pass the dequeueSelector fn to createReleaseQueue as the last argument (as you are already presuming in this test, but the implementation doesn't have it).

Then basically the test becomes more reasonable:

await releaseQueue(actionQueue); expect(mockDispatch).toHaveBeenCalledTimes(0);

Yeah that's the correct implementation, I'll address this 👍

rgommezz · 2019-10-18T00:14:59Z

test/reducer.test.js

@@ -383,3 +406,11 @@ describe('networkSelector', () => {
    });
  });
 });
+
+describe('network reducer config', () => {


What's the purpose of this test?

I think this is a leftover from previous code, I'll remove it. Good catch 👍

emanueleDiVizio · 2019-10-18T08:25:12Z

I've addressed comments 👍
On a side note, I believe I've found a way to stop the queue with the deselector even while it's being released. See code for releaseQueue on df12a54 for more detail.
Only thing, I wasn't able to test this behaviour as I couldn't get the state to change while the queue is being released. I've tried using Promise.all([]) but didn't have any success. It's worth noting that there's no test for the queue being stopped while being released even with the queue semaphore, as I've encountered the same trobule.
@rgommezz could you look into df12a54? Maybe we should think of a way to test the queue being paused midway...

rgommezz

Thanks for the patience and great work @emanueleDiVizio! This is ready to be shipped. I'll merge it later on and will publish a new version on npm. Don't worry about that test case scenario for now, it's good we are back at 100% coverage! :D

emanueleDiVizio · 2019-10-18T12:23:28Z

Yuppie! Thanks for the great support @rgommezz!
On a final note, I believe that dequeueSelector now works even while the queue is being released, but I haven't been able to test it in a real scenario as we moved away from this implementation in our codebase.
@MakhouT since I believe you're planning to test it, would you mind testing that deselector and semaphore work while the queue is being released? We might want to update the docs if that's the case 👍

rgommezz · 2019-10-20T19:08:16Z

@all-contributors please add @emanueleDiVizio for code and doc

allcontributors · 2019-10-20T19:08:26Z

@rgommezz

I've put up a pull request to add @emanueleDiVizio! 🎉

MakhouT · 2019-10-20T22:02:36Z

Nice work guys! Unfortunately I wasn't able to test it yet, I'll try to get to it ASAP, as we also need this in our app. Will provide feedback when it got tested from my side.

emanueleDiVizio added 3 commits October 1, 2019 09:39

Implement queue sempahore mechanism

4a6c1ea

Add config for shouldQueueStartAutomatically

1513830

Export queueSemaphoreChange action

d556be9

emanueleDiVizio mentioned this pull request Oct 1, 2019

Configure whether or not actionQueue should be executed #160

Closed

MakhouT mentioned this pull request Oct 2, 2019

Is it possible to delay offline queue flush #174

Closed

Implement queue deselector

9943f2a

Merge branch 'feature/queue-semaphore' into develop

59244e6

rgommezz requested changes Oct 3, 2019

View reviewed changes

src/redux/actionCreators.js Outdated Show resolved Hide resolved

src/redux/actionTypes.js Outdated Show resolved Hide resolved

src/redux/actionCreators.js Show resolved Hide resolved

emanueleDiVizio added 2 commits October 9, 2019 11:37

Refactor according to community suggestions

0247f79

rename action creator

382bbd0

emanueleDiVizio added 2 commits October 9, 2019 11:59

export action creators

dbb3450

Fix shouldDequeue boolean

46ddd58

Restore correct queue deselctor

7471729

rgommezz requested changes Oct 16, 2019

View reviewed changes

Remove yarn.lock and regroup sholdDeque expression

e561c2d

emanueleDiVizio and others added 3 commits October 16, 2019 15:37

Implement correct check in didComeBackOnline

d0393a6

Update docs

1f0c043

Merge branch 'master' into feature/queue-semaphore

2fc12b5

emanueleDiVizio added 2 commits October 16, 2019 17:10

Add SEMAPHORE_COLOR constant

eb3c380

Merge branch 'feature/queue-semaphore' of github.com:evrythng/react-n…

d4e4669

…ative-offline into feature/queue-semaphore

Fix didQueueResume and didComeBackOnline functions, bring test covera…

7e5ddae

…ge to 100%

rgommezz requested changes Oct 18, 2019

View reviewed changes

Polish code and improve tests

df12a54

rgommezz approved these changes Oct 18, 2019

View reviewed changes

emanueleDiVizio changed the title ~~[WIP] - Feature/queue semaphore~~ Feature/queue semaphore Oct 18, 2019

emanueleDiVizio changed the title ~~Feature/queue semaphore~~ Implement control mechanism for queue release Oct 18, 2019

rgommezz merged commit 35c48b8 into rgommezz:master Oct 20, 2019

allcontributors bot mentioned this pull request Oct 20, 2019

docs: add emanueleDiVizio as a contributor #229

Merged

	shouldDequeueSelector: (state: State) => boolean = state => state.conditionToReleaseQueue
	shouldDequeueSelector: (state: RootReduxState) => boolean = () => true

		@@ -360,6 +361,9 @@ By default it's configured to intercept actions for fetching data following the

		`queueReleaseThrottle`: waiting time in ms between dispatches when flushing the offline queue. Useful to reduce the server pressure when coming back online. Defaults to 50ms.

		`shouldDequeueSelector`: state selector used to control if the queue should be released on connection change. Returning `true` releases the queue, returning `false` prevents queue release. Note, if the result of `shouldDequeueSelector` changes while the queue is being released, the queue will not halt. If you want to halt the queue while is being released, please see relevant FAQ section.

	isQueuePaused: semaphoreColor === 'RED',
	isQueuePaused: semaphoreColor === SEMAPHORE_COLOR.RED,

Implement control mechanism for queue release #225

Implement control mechanism for queue release #225

Conversation

emanueleDiVizio commented Oct 1, 2019 • edited by rgommezz

Motivation

Test plan

melvynhills commented Oct 1, 2019 • edited

emanueleDiVizio commented Oct 2, 2019

emanueleDiVizio commented Oct 2, 2019

rgommezz left a comment • edited

Choose a reason for hiding this comment

melvynhills commented Oct 3, 2019

rgommezz commented Oct 3, 2019 • edited

emanueleDiVizio commented Oct 7, 2019

emanueleDiVizio commented Oct 9, 2019 • edited

emanueleDiVizio commented Oct 9, 2019

emanueleDiVizio commented Oct 12, 2019

emanueleDiVizio commented Oct 12, 2019

MakhouT commented Oct 12, 2019

emanueleDiVizio commented Oct 14, 2019 • edited

MakhouT commented Oct 14, 2019

emanueleDiVizio commented Oct 14, 2019

rgommezz commented Oct 16, 2019

rgommezz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommezz Oct 16, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanueleDiVizio Oct 17, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanueleDiVizio commented Oct 16, 2019

emanueleDiVizio commented Oct 16, 2019

rgommezz commented Oct 16, 2019 • edited

emanueleDiVizio commented Oct 17, 2019

emanueleDiVizio commented Oct 17, 2019

rgommezz left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emanueleDiVizio commented Oct 18, 2019 • edited

rgommezz left a comment

Choose a reason for hiding this comment

emanueleDiVizio commented Oct 18, 2019

rgommezz commented Oct 20, 2019

allcontributors bot commented Oct 20, 2019

MakhouT commented Oct 20, 2019

emanueleDiVizio commented Oct 1, 2019 •

edited by rgommezz

melvynhills commented Oct 1, 2019 •

edited

rgommezz left a comment •

edited

rgommezz commented Oct 3, 2019 •

edited

emanueleDiVizio commented Oct 9, 2019 •

edited

emanueleDiVizio commented Oct 14, 2019 •

edited

rgommezz Oct 16, 2019 •

edited

emanueleDiVizio Oct 17, 2019 •

edited

rgommezz commented Oct 16, 2019 •

edited

rgommezz left a comment •

edited

emanueleDiVizio commented Oct 18, 2019 •

edited