Skip to content
This repository has been archived by the owner on Oct 24, 2022. It is now read-only.

Improves calculation for new subscription delivery count #129

Merged
merged 1 commit into from
Mar 31, 2015

Conversation

richardrodgers
Copy link
Collaborator

by subtracting any items already transferred
Closes #124

by subtracting any items already transferred
Closes #124
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 18.41% when pulling 30e9bd5 on 124_newitem_count into ff456e3 on master.

@JPrevost
Copy link
Member

I think this may not quite take us as far as it could as it does not account for items already in the Hold queue... but then again, maybe it shouldn't? I haven't decided what makes more sense.

The example have was to Subscribe as Review to one Institutional Affiliation for an Item with a ton of Institutional Affiliations, then pull up another Institutional Affiliations Topic for that same Item that also had at least one additional Item associated with this second Topic. The Subscription badge suggested I'd get two new Items if I subscribed but my review count actually only increased by one.

If we decide it makes sense to also account for what is currently in the Hold queue, adding this to the query would work:

and not exists (
  select 1 from hold
  where subscriber_id = {sub_id}
  and item_id = item_topic.item_id
)

If we don't care about the hold queue (and like I said I'm not sure if we do), then 👍

@richardrodgers
Copy link
Collaborator Author

Hmm, interesting point. I'm not sure we ever explicitly established the complete semantics of a hold.
My read was that if one performed any operation (such as subscribing to a topic that belonged to the item) that would result in a delivery, it implicitly released the hold. But other views are certainly reasonable: e.g. that a hold actually isolates the item until an explicit hold action frees it.

@JPrevost
Copy link
Member

In my scenario I was imagining both Topic Subscriptions having Review Interests... so both should add Items to the queue, but since they have overlapping Items the Subscribe badge is just not quite accurate.

From your comment, it is now clear that it does get muddy (ha!) if there are Items in the Hold queue and the new Subscription is a Deliver Interest. The count in that case should probably not take into account the Hold queue as I think it probably does make sense to explicitly release/clear the Hold.

Maybe we just need a new ticket to review the expected flow for new Subscriptions in relation to the Hold queue?

@richardrodgers
Copy link
Collaborator Author

Yea, that makes sense. It might just be a resolved with a brief meeting/analysis in which we walk through each case and ask if the system behavior is reasonable....

@JPrevost
Copy link
Member

👍

@JPrevost
Copy link
Member

FWIW, I think this is merge ready but maybe with a new ticket to remind us to sit down and work out how some of the fringe cases should be handled.

richardrodgers pushed a commit that referenced this pull request Mar 31, 2015
Improves calculation for new subscription delivery count
@richardrodgers richardrodgers merged commit f6e865e into master Mar 31, 2015
@richardrodgers richardrodgers deleted the 124_newitem_count branch March 31, 2015 16:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve delivery calculation for subscriptions
3 participants