Skip to content

feat: display items by group #149

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

Merged
merged 1 commit into from
Apr 25, 2025
Merged

feat: display items by group #149

merged 1 commit into from
Apr 25, 2025

Conversation

0x2E
Copy link
Owner

@0x2E 0x2E commented Apr 25, 2025

Screenshot 2025-04-25 at 16 26 02

Fix #85

@0x2E 0x2E requested a review from Copilot April 25, 2025 08:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a new group filter feature for displaying items by group. Key changes include:

  • Adding a new GroupID query parameter in the request and item filter structures.
  • Updating the database query to join the feeds table for group filtering.
  • Enhancing the frontend to incorporate the new group filtering in the route and API call.

Reviewed Changes

Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/item_form.go Added GroupID field to the request struct.
server/item.go Passed the GroupID to the item filter in the List method.
repo/item.go Updated the query logic to join the feeds table and filter items by group.
frontend/src/routes/(authed)/groups/[id]/+page.ts Integrated group filtering in the page load function.
frontend/src/lib/api/item.ts Updated the listItems response type to include the content field as per the new Item type.
Files not reviewed (4)
  • frontend/src/lib/components/ItemList.svelte: Language not supported
  • frontend/src/lib/components/Pagination.svelte: Language not supported
  • frontend/src/lib/components/Sidebar.svelte: Language not supported
  • frontend/src/routes/(authed)/groups/[id]/+page.svelte: Language not supported

Unread *bool
Bookmark *bool
}

func (i Item) List(filter ItemFilter, page, pageSize int) ([]*model.Item, int, error) {
var total int64
var res []*model.Item
db := i.db.Model(&model.Item{})
db := i.db.Model(&model.Item{}).Joins("JOIN feeds ON feeds.id = items.feed_id")
Copy link
Preview

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

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

The explicit join on the feeds table is introduced to support the group filtering, but the subsequent use of Preload('Feed') may result in redundant or unexpected data loading. Consider reviewing whether both the join and preload are necessary, or if they can be consolidated to ensure efficient querying.

Suggested change
db := i.db.Model(&model.Item{}).Joins("JOIN feeds ON feeds.id = items.feed_id")
db := i.db.Model(&model.Item{}).Joins("JOIN feeds ON feeds.id = items.feed_id").
Select("items.*, feeds.id AS feed__id, feeds.name AS feed__name, feeds.group_id AS feed__group_id")

Copilot uses AI. Check for mistakes.

@0x2E 0x2E merged commit 5c98073 into main Apr 25, 2025
1 check passed
@0x2E 0x2E deleted the group_items branch April 25, 2025 08:29
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.

[FR] Combine feeds in group feed
1 participant