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

Changed notification limit from 100000 to 100 for Inbox and spam page #1841

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

mishramonalisha76
Copy link
Collaborator

@mishramonalisha76 mishramonalisha76 commented Sep 3, 2024

Pull Request Template

Ticket Number

Description

  • Problem/Feature:Changed notification limit from 100000 to 100 for Inbox and spam page

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe):

Checklist

  • Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
    • Quick PR label added
  • Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
    • DO NOT Merge PR label added

Frontend Guidelines

Build & Testing

  • No errors in the build terminal
  • Engineer has tested the changes on their local environment
  • Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

  • Before: Explain the previous behavior
image
  • After: What's changed now
image

Additional Context

Review & Approvals

  • Self-review completed
  • Code review by at least one other engineer
  • Documentation updates if applicable

Notes

@mishramonalisha76 mishramonalisha76 linked an issue Sep 3, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Sep 3, 2024

In the file src/segments/Inbox.tsx:

  1. In the fetchAllNotif function, there seems to be a misplaced object definition. The object properties should be wrapped in curly braces. It should be corrected as follows:

    const fetchAllNotif = async () => {
      setLoadFilter(true);
      try {
        const results = await userPushSDKInstance.notification.list('INBOX', {
          limit: 100,
          page: page,
          raw: true,
        });
        setNotif(parsedResponse);
        setLoadFilter(false);
      } catch (err) {
        console.error(err);
      }
    }
  2. The handlePagination function is missing a closing brace and a closing parenthesis for the if condition. It should be fixed as follows:

    const handlePagination = async () => {
      if (filter) {
        setLimit(limit + 10);
      } else {
        loadNotifications();
      }
    }

In the file src/segments/Spam.tsx:

  1. In the fetchAllNotif function, one closing parenthesis is missing after the raw: true property. It should be added as follows:

    const results = await userPushSDKInstance.notification.list('SPAM', {
      limit: 100,
      page: 1,
      raw: true,
    });
  2. The commented-out dispatch function in the fetchAllNotif function should be considered for uncommenting based on the application's requirements.

  3. In the fetchLatestNotifications function call within the useEffect hook, there seems to be a missing closing brace } after loadNotifications();.

Apart from these issues, the logic and code structures present in both files seem to be fine.

All looks good.

Copy link

github-actions bot commented Sep 3, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-09-04 11:51 UTC

Copy link

github-actions bot commented Sep 4, 2024

All looks good.

@mishramonalisha76
Copy link
Collaborator Author

Screen.Recording.2024-09-04.at.4.30.57.PM.mov

Its a bit faster in other wallets as compared to the wallet address mentioned in the video @rohitmalhotra1420

@rohitmalhotra1420 rohitmalhotra1420 merged commit ab30af9 into main Sep 4, 2024
2 checks passed
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.

Fix the page limit of notification listing
2 participants