Feature #3338

[Workbench] Show number of items in each tab of the project page.

Added by Peter Amstutz about 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Phil Hodgson
Category:
Workbench
Target version:
Start date:
09/11/2014
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

It should show the number of items in each tab on the project page in workbench, so I know which tabs have contents and which tabs are empty. E.g.

Data Collections (1) Jobs and Pipelines (6) Pipeline templates (1) Subprojects (0) Other objects (0)

Subtasks

Task #3854: Implement simple, generic way to get #count for each paneResolvedPhil Hodgson

Task #3860: Review 3338-tab-countsResolvedPhil Hodgson

Associated revisions

Revision 58ccc8f1
Added by Phil Hodgson almost 5 years ago

Merge branch '3338-tab-counts' refs #3338

History

#1 Updated by Peter Amstutz about 5 years ago

  • Subject changed from Show number of items in each tab of the project page. to [Workbench] Show number of items in each tab of the project page.
  • Description updated (diff)
  • Category set to Workbench

#2 Updated by Tim Pierce about 5 years ago

  • Story points set to 0.5

#3 Updated by Radhika Chippada about 5 years ago

  • Target version set to Arvados Future Sprints

#4 Updated by Ward Vandewege almost 5 years ago

  • Target version changed from Arvados Future Sprints to 2014-09-17 sprint

#5 Updated by Tom Clegg almost 5 years ago

  • Assigned To set to Phil Hodgson

#6 Updated by Phil Hodgson almost 5 years ago

So I like what I've done so far. To finish it off - if this approach meets with approval - we would make it so that any operation that would change the count of one of the tabs would have as a side effect a $.get to the tab_counts url in the data attribute of the nav-tabs, which would update the counts in each tab's title.

#7 Updated by Brett Smith almost 5 years ago

Reviewing 893f659

  • You could make tab_counts a little more predictable by using contents.items_available rather than contents.count. This tells you how many results the API server has, even it they didn't all fit in the response.
  • As the guy who added @user_is_manager to this controller, if you're feeling gung-ho about moving this is a before_filter, I think that'd be a nice improvement. I don't think it's necessary for this story, though, if you'd rather put it off. Your call.

Thanks.

#8 Updated by Phil Hodgson almost 5 years ago

Done, as you've recommended.

Now, the thing is, if we like this approach, we have to do another iteration to properly complete this story. Unless there is some existing hook I don't know about, we'll want to find all the various AJAX actions that could alter these counts and have them trigger an event that we listen for. Right now it only listens for document ready. Am I thinking along the right lines?

#9 Updated by Brett Smith almost 5 years ago

Phil Hodgson wrote:

Now, the thing is, if we like this approach, we have to do another iteration to properly complete this story. Unless there is some existing hook I don't know about, we'll want to find all the various AJAX actions that could alter these counts and have them trigger an event that we listen for. Right now it only listens for document ready. Am I thinking along the right lines?

I think you're right that that's the next step. I don't know if it's strictly necessary before you merge your current work—defining the exact scope of story requirements is Tom's job. This bug report seems primarily concerned with "tell me if there are zero items so I don't bother clicking the tab," so I could see a case for having a separate follow-up story.

If you do go ahead, the only action I'm aware of that changes the count dynamically is the delete icon on each row. Creating items usually whisks you off to the page for the new thing, so most of the other obvious candidates are out.

#10 Updated by Phil Hodgson almost 5 years ago

Well if the only places to track changes are on deletions then the job's pretty easy, so I just did it now and committed. I added a javascript event called "count-change" which gets fired from destroy.js.erb and remove_items.js.erb. It will be easy to add triggers elsewhere if it turns out other actions can change the count.

If it all still seems sane, I can go ahead and merge.

#11 Updated by Phil Hodgson almost 5 years ago

  • Status changed from New to In Progress

#12 Updated by Brett Smith almost 5 years ago

  • Status changed from In Progress to New

Reviewing 210d762

Unfortunately, you can't add the user_is_manager code to find_object_by_uuid, because that filter wants to return the value of the else super above this. I think it's appropriate to create a new filter for this.

Moving the code to a filter makes the comment above the Sharing pane_list entry obsolete.

There's a character of trailing whitespace in the new controller comments, which are forbidden by our style guide. Please remove it. (git diff --check can help you track these down.)

Thanks.

#13 Updated by Phil Hodgson almost 5 years ago

Okay, I've created a new before_filter. I was overly concerned about before_filter ordering, which behaves as we would like and expect (ApplicationController before_filters go first). Rather than specifying the same :except list for the new before_filter, I just made it conditional on @object being set, which is all that it really cares about. The Workbench test suite passes.

I also addressed your other observations. Thanks for your patience!

#14 Updated by Tom Clegg almost 5 years ago

  • Status changed from New to In Progress

#15 Updated by Brett Smith almost 5 years ago

Phil Hodgson wrote:

I also addressed your other observations. Thanks for your patience!

Thank you. 3ef64886 looks good to merge, so go for it.

#16 Updated by Ward Vandewege almost 5 years ago

  • Target version changed from 2014-09-17 sprint to 2014-10-08 sprint

#17 Updated by Phil Hodgson almost 5 years ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF