-
Notifications
You must be signed in to change notification settings - Fork 992
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
Allow dataset attribute changes without refreshing the page #4779
Allow dataset attribute changes without refreshing the page #4779
Conversation
6dff0ab
to
9a7150f
Compare
a7b8c19
to
470d1ee
Compare
'name' : name, | ||
'label' : spec.desc, | ||
'value' : attributes.value, | ||
'readonly' : spec.get('readonly') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't violate our linting because we added exceptions for Carl - but I think a lot of us would really prefer if we stuck closer to the flake8 guidelines and not added these extra spaces to align the :
. I'm fine merging as is but can you not do this next time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged with dev and launched a Jenkins Selenium test for this branch here https://jenkins.galaxyproject.org/view/All/job/jmchilton-selenium/183/console. |
Tests are green in dev for me https://jenkins.galaxyproject.org/view/All/job/jmchilton-selenium/183/console. I'm +1 on this. Does this need to be backported to 17.09? |
@jmchilton I am in the middle of testing, please give me a few |
Crap - sorry @martenson. |
Thanks for the review. I'll follow up if we discover issues. |
It seems unfortunate that this get_edit request is always getting 100kB response due to the listing of all dbkeys and all file_types encompassed in its body with no chance of caching this response. But that is also the case before this PR so I will open a new issue for it. |
This PR follows up on #4334. Thanks a lot for you contributions @anuprulez. This PR moves forward and separates the controller
edit
endpoint for dataset attribute editing intoget_edit
andset_edit
.get_edit
returns a dictionary to build the forms,set_edit
updates attributes or triggers conversion operations. The forms are re-rendered without page refreshs and interact in a unified fashion with the backend.Additionally this PR fixes some minor issues:
Currently multiple roles in the permission section are not displayed properly and some links in the history panel are broken, see https://github.com/galaxyproject/galaxy/pull/4779/files#diff-c9603956a927d37ee077b49b291e2cd4L307. Other than that initial warnings are displayed in readonly text input fields, but this PR displays them as regular messages at the top of the view instead. Also editable and readonly permissions are displayed using the same form, an additional custom form is not needed anymore.