Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1582 v3 for subscribe #1342

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

1582 v3 for subscribe #1342

wants to merge 2 commits into from

Conversation

akp111
Copy link
Collaborator

@akp111 akp111 commented Jun 6, 2024

Fixes Issue

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

Copy link

github-actions bot commented Jun 6, 2024

In the code provided, there are some mistakes and improvements needed. Here are the issues found in the code:

  1. In the file packages/restapi/src/lib/channels/signature.helpers.ts:

    • In the getSubscriptionMessageV2 function, there is a missing comma after the userSetting?: string | null parameter declaration.
    • The conditional check should be if (action === 'Subscribe') instead of if (action == 'Subscribe').
    • The else block in the getSubscriptionMessageV2 function is empty. You may need to handle the logic for the Unsubscribe action.
    • In the getSubscriptionMessageV3 function, the Math.floor(Date.now() / 1000) timestamp assignment is not correctly placed.
    • In the getTypeInformation function, the structure should include both Subscribe and Unsubscribe keys within the conditional block.
  2. In the files packages/restapi/src/lib/channels/subscribeV3.ts and packages/restapi/src/lib/channels/unsubscribeV3.ts:

    • There are parts where the code is cut off with .... Check if the missing code could affect the logic.
    • In the unsubscribeV3 function, the typo UnSubscribeOptionsV2Type should be changed to UnsubscribeOptionsV2Type.
  3. In the file packages/restapi/src/lib/pushNotification/notification.ts:

    • In the subscribe function, there is a missing closing parenthesis after pCAIP10ToWallet(channel);.
    • The condition if (!validateCAIP(channel)) is missing a closing curly brace for the block.
    • In the unsubscribe function, there is a missing opening curly brace for the block after the parameter declaration.

After making the necessary corrections, please ensure to test the code thoroughly to confirm all functionalities are working as expected.

Let me know if you need further assistance with these corrections.

@akp111 akp111 added this to the M1 milestone Jun 6, 2024
@mohammeds1992 mohammeds1992 modified the milestones: M1, M2 Jun 14, 2024
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