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

feat: marge type and source #733

Merged
merged 1 commit into from
Dec 25, 2024
Merged

feat: marge type and source #733

merged 1 commit into from
Dec 25, 2024

Conversation

sajdakabir
Copy link
Member

@sajdakabir sajdakabir commented Dec 25, 2024

What did you ship?

Fixes:

  • #XXX (GitHub issue number)
  • MAR-XXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Checklist:

  • I have self-reviewed the code (A decent size PR without self-review might be rejected)
  • I pinky swear that my codes gonna work as I have testing every possible scenario.
  • I ignored Coderabbit suggestion because it does not make any sense.
  • I took Coderabbit suggestion under consideration as some of it makes sense.
  • I have commented my code, particularly in hard-to-understand areas.

OR:

  • shut up and let me cook.

Important

Add feature to filter items by both type and source, updating relevant controllers and routes.

  • Behavior:
    • Replaces getItemsByType with getItemsByTypeAndSource in item.service.js to filter items by both type and source.
    • Updates getItemsByTypeController to getItemsByTypeAndSourceController in item.controller.js to handle new query parameters type and source.
    • Adds new route /items/ in common.route.js to use getItemsByTypeAndSourceController.
  • Renames:
    • Renames getItemsByType to getItemsByTypeAndSource.
    • Renames getItemsByTypeController to getItemsByTypeAndSourceController.

This description was created by Ellipsis for 06a2437. It will automatically update as commits are pushed.

@sajdakabir sajdakabir merged commit 54f5dae into preview Dec 25, 2024
5 of 6 checks passed
@sajdakabir sajdakabir deleted the testing-pr branch December 25, 2024 12:55
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 06a2437 in 1 minute and 12 seconds

More details
  • Looked at 104 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. apps/backend/src/controllers/lib/item.controller.js:1
  • Draft comment:
    Typo in function name 'getAllItemsByBloack'. Consider renaming it to 'getAllItemsByBlock'. This typo is also present in the import/export statements.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. apps/backend/src/routers/lib/common.route.js:28
  • Draft comment:
    Typo in function name 'getAllitems'. Consider renaming it to 'getAllItems'. This typo is also present in the import/export statements.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.
3. apps/backend/src/services/lib/item.service.js:421
  • Draft comment:
    Avoid calling new Date() multiple times. Store the result in a variable to improve performance and readability. This is also applicable in other parts of the code where new Date() is used repeatedly.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_6VbvL9bTz0xeuZxA


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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.

1 participant