Skip to content

Web SDK Refactor: Operation Repo Rework#1264

Merged
fadi-george merged 6 commits into
web-refractorfrom
fg/operation-repo
Apr 14, 2025
Merged

Web SDK Refactor: Operation Repo Rework#1264
fadi-george merged 6 commits into
web-refractorfrom
fg/operation-repo

Conversation

@fadi-george

@fadi-george fadi-george commented Apr 10, 2025

Copy link
Copy Markdown
Contributor

Description

This is the start of the Web SDK rework which can be read here.

With the goal of solving issues with having correct commands (addTag) apply to the correct user (login) in the right order.

This pr focuses on porting on the Android SDK OperationRepo Class to typescript.

1 Line Summary

  • Refactor OperationRepo to be more similar to Android SDK implementation

Details

  • Ports OperationRepo code from Android SDK to typescript
  • Adds new uuid package for setting the operation ids
  • Avoided using LoopWaiterMessage , WaiterWithValue, Waiters as it doesnt seem needed but I may revisit this
  • Likewise did not implement that relied on waiters/wake logic like forceExecuteOperations, waitForNewOperationAndExecutionInterval, delayForPostCreate, delay (already a separate util) methods
  • Did not implement use configModelStore and time (since we have Date) in the constructor. The configModelStore holds some constants that I extracted out but can revisit implementing in later prs
  • Skipped some tests since operation repo caused them to break

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 Reviewable


@fadi-george fadi-george changed the title Operation Repo Rework Web SDK Refactor: Operation Repo Rework Apr 11, 2025
const OP_REPO_EXECUTION_INTERVAL = 5000;
const OP_REPO_POST_CREATE_RETRY_UP_TO = 60_000;

export class NewRecordsState {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment to refer to the NewRecordsState file in the android SDK. Either that, or copy the source-level comments from the file.

I would recommend doing this for each of the classes/functions you've defined below as makes it easier to understand the individual components more easily.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add references to those.

Comment thread src/core/operationRepo/OperationRepo.ts Outdated
let highestRetries = 0;
switch (response.result) {
case ExecutionResult.Success:
// Remove operations from store and wake waiters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update this comment if we're not using waiters

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Comment thread src/core/operationRepo/OperationRepo.ts Outdated
} catch (e) {
Log.error(`Error attempting to execute operation: ${ops}`, e);

// On failure remove operations from store and wake waiters

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment here about clarity

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@fadi-george fadi-george merged commit e3b320d into web-refractor Apr 14, 2025
@fadi-george fadi-george deleted the fg/operation-repo branch April 14, 2025 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants