Idea #1970
closedCreate groups and use them like project folders to manage objects in Workbench.
Added by Tom Clegg almost 11 years ago. Updated over 10 years ago.
Description
- All readable folders show up in left nav section
- reasonable max height, scroll if needed
- type in "filter" box to filter (live in browser, not ajax)
- "New folder" button → pop-up → refresh the folder list
- "Folders" page lists my folders, and folders shared with me (either mine are at the top, or I can filter all/sharedwithme/justmine)
- Rename and delete can be done here
- "Show folder" page
- Adding and removing objects from a folder
- "Add all to..." button on existing "selections" drop-down → select a writable folder from a list
- "Add to..." drop-down at top of object pages
- "Add to..." drop-down next to each object on index pages
- "Remove from folder" button next to each object in folder view
- Refuse to delete a group if it is the owner_uuid of any object
- Add a "group_class" attribute. (Set to "folder" for folders.)
Files
Screenshot from 2014-04-14 16_19_58.png (92.5 KB) Screenshot from 2014-04-14 16_19_58.png | groups#show mockup | Tom Clegg, 04/14/2014 04:20 PM | |
Screenshot from 2014-05-01 07_50_40.png (115 KB) Screenshot from 2014-05-01 07_50_40.png | Tom Clegg, 05/01/2014 10:52 AM |
Updated by Tom Clegg almost 11 years ago
- Subject changed from Create, browse, display, and share groups/projects in Workbench (sets of collections, pipelines, etc) to Create, browse, display, set-default, and share groups/projects in Workbench (sets of collections, pipelines, etc)
Updated by Tom Clegg almost 11 years ago
- Target version set to 2014-05-07 Storing and Organizing Data
Updated by Tom Clegg almost 11 years ago
- Target version changed from 2014-05-07 Storing and Organizing Data to 2014-03-05 Data management
Updated by Tom Clegg almost 11 years ago
- Subject changed from Create, browse, display, set-default, and share groups/projects in Workbench (sets of collections, pipelines, etc) to Create groups and use them like folders to manage objects in Workbench.
Updated by Tom Clegg almost 11 years ago
- Subject changed from Create groups and use them like folders to manage objects in Workbench. to Create groups and use them like project folders to manage objects in Workbench.
Updated by Tom Clegg over 10 years ago
- Target version changed from 2014-03-05 Data management to 2014-05-07 Storing and Organizing Data
Updated by Tom Clegg over 10 years ago
- Target version changed from 2014-05-07 Storing and Organizing Data to 2014-04-16 Dev tools and data/resource management
Updated by Tom Clegg over 10 years ago
- Target version changed from 2014-04-16 Dev tools and data/resource management to 2014-05-07 Storing and Organizing Data
Updated by Tom Clegg over 10 years ago
- Target version deleted (
2014-05-07 Storing and Organizing Data) - Release changed from 4 to 8
Updated by Tom Clegg over 10 years ago
- Target version set to 2014-05-07 Storing and Organizing Data
Updated by Tom Clegg over 10 years ago
Updated by Tom Clegg over 10 years ago
Updated by Brett Smith over 10 years ago
I'm looking at c52a78b3, and I'm calling it now: Tom wins this sprint review. No other demo could possibly compare to this branch for raw eye candy. "Oh, we can run Crunch jobs in Docker containers now? That's nice. Me? Oh, I just MADE THE WORKBENCH INTERFACE GLORIOUS. BEHOLD my expansive collection of beautiful screenshots!" I can hear the business team ohh-ing and ahh-ing already.
Seriously, this is very impressive work. Kudos. My comments feel even more nit-pickier than usual by comparison (and really, they're mostly questions for understanding rather than suggestions for improvements), but here goes anyway:
- ActionsController calls
#send(sym.to_s)
. Sending symbols is allowed (and I think preferred), so you can omit.to_s
. - FoldersController#index returns @object. What is that for index methods?
- The first folder integration test has no assertion. Could you
assert find('.panel').has_text?('I just edited this.')
? - Maybe related, I'm curious why you removed some of the intermediate assertions in the existing integration tests.
- I'm not wild about the cloud icon for virtual machines in the navigation sidebar. Virtual machines may be in the cloud, but they are not themselves the cloud. Is there a generic computer/workstation icon? A work desk might make sense, too, since the virtual machine is where people work on Crunch scripts.
Thanks much.
Updated by Tom Clegg over 10 years ago
Brett Smith wrote:
Seriously, this is very impressive work. Kudos. My comments feel even more nit-pickier than usual by comparison (and really, they're mostly questions for understanding rather than suggestions for improvements), but here goes anyway:
Thanks!
- ActionsController calls
#send(sym.to_s)
. Sending symbols is allowed (and I think preferred), so you can omit.to_s
.
Indeed, fixed.
- FoldersController#index returns @object. What is that for index methods?
Hm, not actually sure why that got there. Removed.
- The first folder integration test has no assertion. Could you
assert find('.panel').has_text?('I just edited this.')
?
Fixed. (I had commented it out pending discovering the magic incantation to reconcile Capybara with x-editable. "visit current_path"
isn't perfect but better than nothing.)
- Maybe related, I'm curious why you removed some of the intermediate assertions in the existing integration tests.
Some of the tests changed from "click the uuid link" to "click the Show button on the row with this uuid". While I was updating them, noticing find()
errors out if the links don't exist, I figured the extra assert
before find().click
doesn't serve any purpose (unless it has a helpful error message).
- I'm not wild about the cloud icon for virtual machines in the navigation sidebar. Virtual machines may be in the cloud, but they are not themselves the cloud. Is there a generic computer/workstation icon? A work desk might make sense, too, since the virtual machine is where people work on Crunch scripts.
Yeah, I'm not super keen on it either. I've changed it to the "terminal" icon, see how that goes. See http://fortawesome.github.io/Font-Awesome/icons/
Latest: 16c58b7
Updated by Brett Smith over 10 years ago
- Status changed from In Progress to New
On 16c58b75
Tom Clegg wrote:
Brett Smith wrote:
- The first folder integration test has no assertion. Could you
assert find('.panel').has_text?('I just edited this.')
?Fixed. (I had commented it out pending discovering the magic incantation to reconcile Capybara with x-editable.
"visit current_path"
isn't perfect but better than nothing.)
You say below that find
errors out if it doesn't find what it's looking for. So won't this always either pass or raise an exception? Or does that only apply when you try to click
the result?
- Maybe related, I'm curious why you removed some of the intermediate assertions in the existing integration tests.
Some of the tests changed from "click the uuid link" to "click the Show button on the row with this uuid". While I was updating them, noticing
find()
errors out if the links don't exist, I figured the extraassert
beforefind().click
doesn't serve any purpose (unless it has a helpful error message).
+1 to helpful messages. My personal feeling is that an assert is a much stronger statement of intent than an uncaught exception. A failed assertion says, "I expected this behavior, and didn't get it." An exception leaves open questions to answer: is this bug in the test code, or in the application? And if it's in the application, is the expected behavior important, or not? So I like redundant assertions because I feel like they help others better interpret the results—e.g., many of my recent controller tests assert_response :success
even though they go on to check things that could only be true after success.
But this is a pretty open-ended style thing, and I can definitely agree that the subject's a little more slippery when you're talking about relatively fine-grained navigation of HTML. That's my two cents, but either way, once the specific test issue above is settled I'm happy (psyched, even) to see this branch merged.
Updated by Brett Smith over 10 years ago
- Status changed from New to In Progress
Updated by Tom Clegg over 10 years ago
Brett Smith wrote:
On 16c58b75
You say below that
find
errors out if it doesn't find what it's looking for. So won't this always either pass or raise an exception? Or does that only apply when you try toclick
the result?
That's true, the word "assert" doesn't actually do anything because failure results in an exception.
- Tried wrapping in "assert_nothing_raised", but that somehow causes
find()
to always fail. - Tried using the "message" option to
find()
, but that feature has been broken/removed from Capybara. Added a comment (in a848b6e) explaining thatfind()
raises an exception if the tested behavior fails.- Added a
find?()
method (in 32ca1dc) to use with assert.
+ assert(find?('.panel', text: 'I just edited this.'),
+ "Description update did not survive page refresh")
Totally on board with improving our testing standards/patterns. In particular, valuable to make more clear in the tests which parts are "setting up the test" (rearrange as needed) and which parts are "behavior actually being tested" (when you touch this code, be careful that you are still testing this behavior!).
Updated by Anonymous over 10 years ago
- Status changed from In Progress to Resolved
- % Done changed from 95 to 100
Applied in changeset arvados|commit:fc4428224984cb71b961d17410205b535153c7f2.