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

fix: changes in list, search and tags #1394

Merged
merged 1 commit into from
Sep 24, 2024
Merged

Conversation

akp111
Copy link
Collaborator

@akp111 akp111 commented Sep 23, 2024

added listing for tags, filter for seach and list, support for old and new response in search

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

added listing for tags, filter for seach and list, support for old and new response in search
Copy link

In the code provided, I found a few issues and suggestions for improvement:

  1. In packages/restapi/src/lib/channels/getAllTags.ts:

    • In the GetAllTagsOptionsType, the filter property is of type string, but it might be more appropriate to define it as one of the specific filter types like "PUSH" or "USER".
  2. In packages/restapi/src/lib/channels/getChannels.ts:

    • In the getChannelsOptionsType, the filter property is defined as a number, but in the implementation, it seems to be used as a string. Make sure the property type matches its usage.
  3. In packages/restapi/src/lib/channels/search.ts:

    • There are multiple ellipses (...) and unfinished comments in the file. These should be reviewed and completed as required.
    • There is a missing closing brace for the search function definition at the end of the file.
  4. In packages/restapi/src/lib/constantsV2.ts:

    • There is a typo in the Notification enum definition, it should be Notification instead of Notifictaion.
    • The SEARCH enum property is not defined in the CONSTANTS object.
  5. In packages/restapi/src/lib/pushNotification/channel.ts:

    • The search function is missing a closing curly brace which will result in a syntax error.
    • Error handling inside the subscribers method seems inconsistent, with some code paths missing appropriate error handling.

Overall, the structure and logic seem fine, but these issues need to be addressed for better code quality and consistency.

If you make the necessary corrections, please let me know so I can re-review the updated code.

@mohammeds1992 mohammeds1992 merged commit 665a96d into main Sep 24, 2024
1 check 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.

2 participants