Feature #4024
closed[Workbench] PipelineInstances#index should scroll down to show older pipelines, and should have a search box (similar to the "pipelines" tab on the "show project" page)
Added by Tom Clegg over 10 years ago. Updated about 10 years ago.
100%
Updated by Ward Vandewege about 10 years ago
- Target version changed from Arvados Future Sprints to 2014-10-29 sprint
Updated by Radhika Chippada about 10 years ago
- Assigned To set to Radhika Chippada
Updated by Radhika Chippada about 10 years ago
- Status changed from New to In Progress
Updated by Tom Clegg about 10 years ago
- Target version changed from 2014-10-29 sprint to 2014-11-19 sprint
Updated by Radhika Chippada about 10 years ago
How do we want to prevent page refresh on the dashboard refreshing the recently finished pipelines panel when the user has scrolled down and viewing "next" page(s)?
Do we want to "disable" page refresh to the panel in that scenario?
Updated by Tom Clegg about 10 years ago
- Category set to Workbench
At 50517f3...
Rather than adding a new route we could use the @/?tab_pane=finished_pipelines (and name the partial _show_finished_pipelines.html).
Was there a reason for changing the "finished pipelines" content from <div class="row">
to a <table><tr>
?
The refresh behavior is pretty awkward here: the entire content area of the dashboard refreshes itself, which causes the "finished" pane to be empty, and then the loading of the finished pane begins. This can be addressed partially by the more recent reload pattern which was introduced for the dashboard(?) and is being further refined in #4084: we can update all of the dashboard panes independently, and remove the auto-update from the container that encloses all of the panes.
This still leaves us with a conflict between infinite scroll and dynamic refresh: we have no way to insert/update rows automatically without clearing the entire result set and starting over from page 1. This will be slow, and even worse, it will cause the browser to scroll automatically to the top every ~15 seconds, which will be even more annoying than the auto-scroll-to-bottom log viewer bug! I think the best solution is to use a client side framework like Angular, but perhaps there is also a more expedient solution.
First thought: Say we provided (some) content updates with a flag instructing the client-side reloader to "find objects with selector X, and use them as replacements for existing elements with the same ID". Any new matching elements could be inserted into the DOM the appropriate order. Dealing correctly with updated content on page 4 would probably require the client to tell the server a range (or a list of UUIDS?) of objects it should render.
Updated by Radhika Chippada about 10 years ago
Discussed this feature with Tom some more. We agreed that instead of adding infinite scrolling to the dashboard panels, we should add infinite scrolling to individual pages such as pipeline_instances page.
Updated by Tom Clegg about 10 years ago
At 7b4fc9f...
The data-infinite-scroller attribute should be a jQuery selector string resolving to the element containing the rows (in this case, the tbody itself) so perhaps:- <tbody data-infinite-scroller="recent-pipeline-instances-scroll" + <tbody data-infinite-scroller="#recent-pipeline-instances" id="recent-pipeline-instances"
- data-filterable-target="table.arv-recent-pipeline-instances tbody" - data-filterable-target="table.arv-recent-pipeline-instances > tbody"
- (Or even use the id of the tbody itself, if you add one as above)
Search/filter shouldn't require a remote form (I don't think we want Rails to do its remote:true ajax stuff here -- as is, the button generates a useless request, the result of which gets ignored somehow). Can it just be an input tag, like the search box in views/application/_choose.html.erb
?
The params[:partial]
handling in ApplicationController
looks like it could fit inside the existing f.json{}
instead of mirroring the logic. (As it stands, non-json with params[:partial] will render nothing at all, which doesn't seem like an important feature to add/preserve.)
I think we should get rid of the "pretend there is no next page if any filters were passed" stuff, and concentrate instead on making next_page_href (and everything else) propagate the filters to the next page correctly.
Isn't it redundant to use both the.html
suffix and formats: [:html]
here?
partial: "show_#{params[:partial]}.html", formats: [:html]
Updated by Tom Clegg about 10 years ago
- Subject changed from [Workbench] Dashboard "recent pipelines" panel should scroll down to show older pipelines, and should have a search box (similar to the "pipelines" tab on the "show project" page) to [Workbench] PipelineInstances#index should scroll down to show older pipelines, and should have a search box (similar to the "pipelines" tab on the "show project" page)
Updated by Radhika Chippada about 10 years ago
Tom, addressed all your feedback. Also, search works with scrolling now for this page. Thanks.
Updated by Radhika Chippada about 10 years ago
Tom, question about apps/workbench/app/views/pipeline_instances/index.html.erb :
Does the "compare" form defined at line 2 work? I selected a couple of instances and still did not see the button. I then removed the "disabled: true, style: 'display: none'", just for testing, and clicking on the button (after selecting 2 or 3) gives Not found error for the URL "https://localhost:3031/pipeline_instances/compare?utf8=%E2%9C%93&commit=Compare+2+or+3+selected". Not sure if it worked before but was broken somewhere along the way or never worked. I don't think I found any tests for it. Let me know if you would like me to file a bug for this.
<%= form_tag({action: 'compare', controller: params[:controller], method: 'get'}, {method: 'get', id: 'compare', class: 'pull-right small-form-margin'}) do |f| %> <%= submit_tag 'Compare 2 or 3 selected', {class: 'btn btn-primary', disabled: true, style: 'display: none'} %> <% end rescue nil %>
Updated by Tom Clegg about 10 years ago
Radhika Chippada wrote:
Does the "compare" form defined at line 2 work?
This seems to have been broken by the tab loading code: the handler wasn't getting attached because the checkboxes weren't present at initial page load time. I've pushed a fix, 6c7bfabc, to your 4024 branch. Unfortunately it interferes with the layout (the search box next to the "compare" button looks a bit odd).
Updated by Tom Clegg about 10 years ago
Radhika Chippada wrote:
Tom, addressed all your feedback. Also, search works with scrolling now for this page. Thanks.
Looks good! Couple of notes:
I suspect this addition is no longer needed -- is that right?+ if params[:search].andand.length.andand > 0 + @select ||= PipelineInstance.columns.map(&:name) + base_search = PipelineInstance.select(@select) + @objects = base_search.where(any: ['contains', params[:search]]). + uniq { |pi| pi.uuid } + end +
- View the pipeline instances page
- Type a string in the search box
- Click one of the pipelines to show the pipeline_instances#show page
- Hit the browser "back" button
- The instances page loads the first page of results
- As soon as the first page is loaded, it erases and reloads again.
I suspect this means the first "first page load" AJAX happens before the filter state is properly set up. I think it would be a good idea to replicate this problem as a failing test case. If the solution isn't apparent, I wouldn't mind poking at it.
Updated by Tom Clegg about 10 years ago
- merging 4388-workbench-update to eliminate javascript errors that were breaking workbench tests
- merging master
- fixing some resulting breakage (related to #3400).
- 06afd90 3400: Do not fetch_multiple_pages in #index actions.
- This caused infinite scroll to get all pages instead of one page at a time.
- 83e73ed 4024: @limit override must happen before find_objects_for_index.
@limit=20
inpipeline_instances#index
was being ignored.
- 5d87bca 3400: Do not fetch API results just for the sake of looking up resource_class.
- This used to cause one unnecessary API call; with 3400 it got much worse, causing an unnecessary
fetch_multiple_pages
- This used to cause one unnecessary API call; with 3400 it got much worse, causing an unnecessary
- 06afd90 3400: Do not fetch_multiple_pages in #index actions.
Updated by Radhika Chippada about 10 years ago
Tom,
- pipeline_instances page with no search string, with a matching search string works. However, if I were to enter a search filter with no matches (basically, some random / junk string), I see "Oops, request failed" error page. It appears that the response from API server seems incomplete / incorrect in this case.
#<NoMethodError: undefined method `+' for nil:NilClass>
/home/radhika/arvados/apps/workbench/app/controllers/application_controller.rb:227:in `next_page_offset'
/home/radhika/arvados/apps/workbench/app/controllers/application_controller.rb:238:in `next_page_href'
/home/radhika/arvados/apps/workbench/app/controllers/application_controller.rb:174:in `block (2 levels) in render_index'
/home/radhika/arvados/apps/workbench/vendor/bundle/ruby/2.1.0/gems/actionpack-4.1.1/lib/action_controller/metal/mime_responds.rb:258:in `call'
/home/radhika/arvados/apps/workbench/vendor/bundle/ruby/2.1.0/gems/actionpack-4.1.1/lib/action_controller/metal/mime_responds.rb:258:in `respond_to'
/home/radhika/arvados/apps/workbench/app/controllers/application_controller.rb:171:in `render_index'
/home/radhika/arvados/apps/workbench/app/controllers/application_controller.rb:214:in `index'
- I updated the integration test that is using "junk" search filter to assert that there is no error in page; this test fails for the time being due to this issue.
Updated by Radhika Chippada about 10 years ago
- Status changed from In Progress to Resolved
- % Done changed from 80 to 100
Applied in changeset arvados|commit:ff7b22c70cd77073d9bdbebac0bf03d43745ed0c.