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

Adds delivery count badge to Subscribe button on topic page #123

Merged
merged 1 commit into from
Mar 19, 2015

Conversation

richardrodgers
Copy link
Collaborator

closes #122

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 18.6% when pulling 0ff9549 on 122_sub_load_UI into 39f98d2 on master.

@JPrevost
Copy link
Member

I think this is good, but have a concern as to whether it does what your intent for this feature is.

My understanding is you want to expose to the Subscriber what will happen when they click a Subscribe button. However, I think there are cases where that won't be accurate. As written this will tell the user the MAX number of Items that will be delivered under a new Subscription, but not necessarily the Actual value due to potential overlapping Items from other Subscriptions. It's even possible that, for example, a new Subscription for an Orcid will deliver nothing at all if the Subscriber has already Subscribed to all of the Institutional Affiliation Names in which that Orcid has ever appeared regardless of whether said Orcid has appeared in a dozen Items since the Subscriber signed up. (Unless I'm misunderstanding what you explained when I opened #119).

@richardrodgers
Copy link
Collaborator Author

The intent here is largely just to explore how the UI should expose contextual data the hub holds. We would refine the calculation as needed. In this case, I think it's not so much overlapping subscriptions or any indeterminacy in the number (max vs actual), since any existing subscriptions would have already been delivered at page render time (put another way, when the button was pushed, the delivery is essentially over) - rather it is the number currently displayed less those already delivered (by any means, including manual deposits). I can certainly add that math now, but I was holding off until we settled a few other things (e.g. API for transfer counts in #115). But thanks for the close read, and I'm more than happy to sit on this until we can sort out these surrounding issues)

@JPrevost
Copy link
Member

Nah, I'd say merge it and we can adjust as our APIs make it easier for sure. 👍

richardrodgers pushed a commit that referenced this pull request Mar 19, 2015
Adds delivery count badge to Subscribe button on topic page
@richardrodgers richardrodgers merged commit 96a2dab into master Mar 19, 2015
@richardrodgers richardrodgers deleted the 122_sub_load_UI branch March 19, 2015 17:41
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.

Indicate size of subscription 'load' in UI
3 participants