Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.07] Hide form instead of removing it to preserve attached listenTo events #2737

Merged

Conversation

Projects
None yet
3 participants
@guerler
Copy link
Contributor

commented Aug 5, 2016

To test this fix execute a regular tool e.g. Add Column with active job resource options or another tool with conditional select fields and observe the server log. In the first case it will display the following error: ValueError: ('No case matched value:', '__job_resource', 'None') after job execution. Due to the attached listenTo event in #2476 the form attempts a refresh after the history changes with newly added job datasets. This refresh fails since the removal of the underlying form dom triggers jquery input element resets. Hiding the form instead of removing it resolves the issue.

@guerler guerler added this to the 16.07 milestone Aug 5, 2016

@guerler guerler force-pushed the guerler:hide_form_on_success branch from 84b6a82 to 1a4b9ec Aug 5, 2016

@guerler guerler added status/review and removed status/WIP labels Aug 5, 2016

@guerler guerler force-pushed the guerler:hide_form_on_success branch from 1a4b9ec to 786da21 Aug 5, 2016

@jmchilton jmchilton merged commit 3f8f9b7 into galaxyproject:release_16.07 Aug 5, 2016

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
api test Build finished. 224 tests run, 0 skipped, 0 failed.
Details
framework test Build finished. 111 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 582 tests run, 0 skipped, 0 failed.
Details
@dannon

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

I was a little uncomfortable with this as a long term solution so I talked to @guerler about this a bit and followup work will obviate the need for this hidden-yet-still-listening approach, and we'll be able to destroy the objects. This is good for an interim fix though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.