Skip to content

Rename to_dict to populate in FormDefintion to fix bug - #16553

Merged
guerler merged 1 commit into
galaxyproject:devfrom
jdavcs:dev_formdef_bugfix
Aug 15, 2023
Merged

Rename to_dict to populate in FormDefintion to fix bug#16553
guerler merged 1 commit into
galaxyproject:devfrom
jdavcs:dev_formdef_bugfix

Conversation

@jdavcs

@jdavcs jdavcs commented Aug 14, 2023

Copy link
Copy Markdown
Member

Old method was overriding the method defined in a mixin (with a different signature) that was called from api.forms

For context, see discussion on backend channel:
The model.FormDefinition.to_dict() method (added in 09/2016 in #3118) takes user, value, and security arguments, which overrides the to_dict definition in the Dictifiable mixin, which takes view and value_mapper as arguments. So, if we call the to_dict method on a FormDefinition object passing it the view/value_mapper arguments, an error will be raised. However, we have (at least) 4 places where we call this method on this object: in galaxy.api.forms it's called in the index, show and create methods with the WRONG arguments (view, value_mapper), and in galaxy.api.users with the CORRECT arguments (user, values, security), which is the ONLY place in lib where it's called with those arguments.

Bug discovered via mypy after adding type annotations in #16434.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Old method was overriding the method defined in a mixin (with a
different signature) that was called from api.forms
@jdavcs
jdavcs requested a review from guerler August 14, 2023 19:52
@github-actions github-actions Bot added this to the 23.2 milestone Aug 14, 2023
@jdavcs

jdavcs commented Aug 14, 2023

Copy link
Copy Markdown
Member Author

test failures unrelated

@guerler
guerler merged commit 983bbfb into galaxyproject:dev Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants