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

include annotation tags for /collections/:key/tags #163

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

abaevbog
Copy link
Contributor

Tags for annotations are counted just like items in numItems field. So, if tag A belongs to an annotation of an attachment, and the attachment itself has tag A, numItems will be 2.

Fixes: #162

@abaevbog abaevbog requested a review from dstillman August 21, 2023 14:36
Copy link
Member

@dstillman dstillman left a comment

Choose a reason for hiding this comment

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

This will cause tags to be returned, but those tags won't actually match any top-level items (e.g., when clicked in the tag selector, which this is for), so I think we'll need to update Items::search() as well, and that's where the itemTopLevel stuff comes in.

JOIN itemAnnotations USING (itemID)
JOIN itemAttachments ON itemAttachments.itemID = itemAnnotations.parentItemID
JOIN collectionItems ON collectionItems.itemID = itemAttachments.sourceItemID
WHERE collectionID=? GROUP BY tagID;";
Copy link
Member

Choose a reason for hiding this comment

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

No semicolon needed

$annotationsSql = "SELECT tagID, COUNT(*) AS numItems FROM tags JOIN itemTags USING (tagID)
JOIN itemAnnotations USING (itemID)
JOIN itemAttachments ON itemAttachments.itemID = itemAnnotations.parentItemID
JOIN collectionItems ON collectionItems.itemID = itemAttachments.sourceItemID
Copy link
Member

Choose a reason for hiding this comment

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

House style: parens around the expression for consistency with USING, even though it's not required for ON

@@ -629,6 +629,24 @@ public function getTagItemCounts() {
foreach ($rows as $row) {
$counts[$row['tagID']] = $row['numItems'];
}
// Fetch the tags of annotations as well
$annotationsSql = "SELECT tagID, COUNT(*) AS numItems FROM tags JOIN itemTags USING (tagID)
Copy link
Member

Choose a reason for hiding this comment

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

JOIN on next line, but I guess we technically don't need tags here or above, so could just remove that.

@abaevbog
Copy link
Contributor Author

abaevbog commented Aug 22, 2023

I see. Now the annotation tags are returned but if we, say, hit /collections/*/items/top?tag=ANNOTATION_TAG, we get an empty response, though we would want to receive the top level item whose attachment's annotation has ANNOTATION_TAG

In Zoteri_Items::search, if /top items are requested,
fetch the top item's itemID during tag filtering as opposed
to the actual item's itemID.
That way, if an attachment has a tag, the top level attachment
would be matches on that tag search query.
@abaevbog
Copy link
Contributor Author

abaevbog commented Aug 22, 2023

When in /top mode, we can do an extra join on topLevelItem to fetch the top level itemID, as opposed to the actual item's itemID, when possible. Then, if there is a child annotation with a tag, /collections/*/items/top?tag=ANNOTATION_TAG will return the top level item for that annotation's tag.

It will do the same for other children like notes or attachments - do we want it? The web library calls /collections/*/items/top?tag=... when a tag is selected, and currently it only returns items in the collection whose tags match the query (so tags of, say, attachments do nothing). The desktop tag filtering does allow us to see the top level item when we click on the attachment's or annotation's tag in the left panel, so maybe it's not a bad thing to add it to web library too?

If we want to keep filtering by all children's tags in /top mode, should we return other children's tags when /collections/*/items/top/tags endpoint is hit too? On the desktop app, I can see my attachments tags in the left panel but they are not returned currently with /collections/*/items/top/tags (which web library calls) , so they do not show up.

@abaevbog abaevbog requested a review from dstillman August 22, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Include annotation tags in <userOrGroupPrefix>/collections/<collectionKey>/tags
2 participants