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

Metadata parameters rely on their own optional setting not on the field attribute #5027

Merged
merged 7 commits into from Nov 28, 2017

Conversation

Projects
None yet
6 participants
@guerler
Contributor

guerler commented Nov 16, 2017

Fixes #4966. The optional attribute of metadata parameters is not parsed to the underlying input field. As a consequence non-optional parameters are shown as optional if the input fields attributes is relied on. The non-optional parameter can then receive an empty string or None which triggers an exception for Range parameters. ping @bgruening. Can you verify that this fixes the issue? Thanks for reporting.

@guerler guerler added this to the 18.01 milestone Nov 16, 2017

@@ -8,7 +8,7 @@
from six import string_types
from galaxy.util import restore_text, unicodify
from galaxy.util import restore_text, unicodify, asbool

This comment has been minimized.

@nsoranzo

nsoranzo Nov 17, 2017

Member
from galaxy.util import (
    asbool,
    restore_text,
    unicodify
)

This comment has been minimized.

@guerler

guerler Nov 17, 2017

Contributor

Thx for the hint @nsoranzo!

@guerler guerler added status/review and removed status/WIP labels Nov 17, 2017

@martenson

This comment has been minimized.

Member

martenson commented Nov 17, 2017

Once confirmed we should make sure to backport this to 17.09

@bgruening

This comment has been minimized.

Member

bgruening commented Nov 17, 2017

It mostly works. If I have a BED file and enter an "Annotation" it gets saved. But I can not delete my Annotation. I can change the Annotation, but not delete it. Other fields I can make empty, like Name or Info.

I can also add a string to "Number of comment lines". Not sure if this is a regression or if this was possible before.

Thanks Sam!

@guerler

This comment has been minimized.

Contributor

guerler commented Nov 17, 2017

Same behavior on 14.10. Apparently annotations are survivors. I can fix that in a follow-up if there is interest in it.

@jmchilton jmchilton merged commit 68795e4 into galaxyproject:dev Nov 28, 2017

7 checks passed

api test Build finished. 315 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 58 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
selenium test Build finished. 100 tests run, 1 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@dpryan79

This comment has been minimized.

Contributor

dpryan79 commented Nov 28, 2017

Is it planned to backport this to 17.09? I've had a couple people mention this issue in the last week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment