Operation repo refactor

[shepherd-l]

Description

1 Line Summary

An operation repo refactor that will help with the implementation of identity verification

Details

• Adds a delta queue to the operation repo to flush all executor deltas at the same time. Now the operation repo will be in charge of enqueuing deltas and passing the delta to its correct executor. Previously, each executor was in charge of enqueuing and flushing their own delta queue.
  • Makes it easier to implement pausing on the operation repo for Jwt when receiving an invalid token.
  • This change also aligns the Web SDK more with the iOS SDK's operation repo implementation.
• Groups push, sms, and email subscriptions under 1 executor. Previously push, sms, and email each had their own subscription executor.
  • Omits creating multiple subscription executors. Unnecessary now that there is only 1 flush from the operation repo that flushes all executors
  • Deprecates the pushSubscription, smsSubscription, and emailSubscription model names for 1 new name: subscriptions. This change requires a database upgrade.
• Adds external id property to operations
  • Will be used to retry operations for users with a valid Jwt token. Accessed through Operation.model.externalId
• Added to following tests below:

Unit Tests:

• v5 to v6 database migration for subscription records. If a user was already logged in v5, the external id is added to each subscription record in v6.
• Login test, where the external id is updated correctly for each model which will be used in deltas/operations.
  • Future work: add logout tests

Systems Affected

• WebSDK
• Backend
• Dashboard

Validation

Tests

Info

Checklist

• All the automated tests pass or I explained why that is not possible
• I have personally tested this on my machine or explained why that is not possible
• I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

• Don't use default export
• New interfaces are in model files

Functions:

• Don't use default export
• All function signatures have return types
• Helpers should not access any data but rather be given the data to operate on.

Typescript:

• No Typescript warnings
• Avoid silencing null/undefined warnings with the exclamation point

Other:

• Iteration: refrain from using elem of array syntax. Prefer forEach or use map
• Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Checklist

• I have included screenshots/recordings of the intended results or explained why they are not needed

---

Related Tickets

---

---

This change is 

[jkasten2]

Does the ExecutorFactory already ensure we don't have duplicate executors? Or if it doesn't take care of it, shouldn't we just omit adding it in the first place?

[rgomezp]

Under this PR, the factory will return the reference to the single static subscription executor three times and be mapped to via the three ModelName keys that correspond to it: pushSubscriptions, emailSubscriptions, and smsSubscriptions

There's probably a cleaner way to do this without hard-coding a check here

[shepherd-l]

Adding on to what rgomezp said:

The ExecutorStore creates an executor for each value in ModelName
And there are 3 names for subscriptions:

1. Identity
2. Properties
3. PushSubscriptions
4. EmailSubscriptions
5. SmsSubscriptions

This PR should ensure that there are no duplicate executors from the ExecutorFactory. But we could omit adding it in the first place by having 1 ModelName for the subscription executor instead. I don't think this would heavy lift but I'm not sure.

[rgomezp]

The ModelName enum is widely used, not just for the executor factory.

You would have to change the way the ExecutorStore indexes the executors to only have 3 keys. But this adds a point of maintenance in the future should we add new supported models.

Is there an issue arising from the differences in behavior between the WebSDK & iOS? Or is this PR aimed at bringing the SDKs to parity to address an outstanding issue?

[shepherd-l]

Having one flush that is controlled by the operation repo would make it easier to implement a JWT feature where we would like to pause the operation repo when we receive an invalid token. And with changing to 1 flush, I didn't think there would be a need to have 3 separate subscription executors for push, sms, email.

[shepherd-l]

But this adds a point of maintenance in the future should we add new supported models.

Could you elaborate on what maintenance would be needed?

You would have to change the way the ExecutorStore indexes the executors to only have 3 keys.

I thought this change would consist of changing the enum and the previous references to use 1 subscription enum. What other things would need to be done?

[rgomezp]

Could you elaborate on what maintenance would be needed?

The executor store store interface looks like

type ExecutorStoreInterface = {
  [key in ModelName]?: OSExecutor;
};

If you change it to say

type ExecutorStoreInterface = {
  identity: OSExecutor;
  properties: OSExecutor;
  subscriptions: OSExecutor;
};

or similar you would have to update in two places should you add new model types. Not a huge deal though.

I thought this change would consist of changing the enum and the previous references to use 1 subscription enum. What other things would need to be done?

The enum isn't just used for creating executors. It's used throughout the user-model SDK.

[jkasten2]

I believe it would instead be changing this code:

export enum ModelName {
  Identity = 'identity',
  Properties = 'properties',
  // TO DO: make singular
  PushSubscriptions = 'pushSubscriptions',
  EmailSubscriptions = 'emailSubscriptions',
  SmsSubscriptions = 'smsSubscriptions',
}

to this:

export enum ModelName {
  Identity = 'identity',
  Properties = 'properties',
  Subscriptions = 'subscriptions',
}

I think this will simply things and means if we add another subscription type we won't have to change this again. We will have to update code that depends on ModelName however, and migrate any existing cached operations to the new name though.

[rgomezp]

I think this will simply things and means if we add another subscription type we won't have to change this again. We will have to update code that depends on ModelName however, and migrate any existing cached operations to the new name though.

That's a big lift. We also rely on the model name to differentiate between subscription models, like in the CoreModuleDirector, OSModel, etc...

[rgomezp]

Under this PR, the factory will return the reference to the single static subscription executor three times and be mapped to via the three ModelName keys that correspond to it: pushSubscriptions, emailSubscriptions, and smsSubscriptions

There's probably a cleaner way to do this without hard-coding a check here

[rgomezp]

Just a couple comments. After this I think LGTM