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

Add form-specific functionality to ModelAdmin #97

Merged
merged 7 commits into from
Mar 21, 2022

Conversation

dwreeves
Copy link
Contributor

@dwreeves dwreeves commented Mar 20, 2022

I added the following attributes to ModelAdmin:

  • form
  • form_base_class
  • form_args
  • form_columns
  • form_excluded_columns
  • form_overrides

All of these attributes emulate their respective functionality in the Flask-Admin API: https://flask-admin.readthedocs.io/en/latest/api/mod_model/

This PR addresses issue #96.

--

Note that I abstracted out the get_{x}_columns() functionality in ModelAdmin. Let me know if you want me to revert this back to being repetitive.


As of writing, there is one thing currently missing from my PR: the scaffolding does not make use of column_labels. This is something I will hope to finish up soon!

Let me know what other changes you want me to make.

@dwreeves
Copy link
Contributor Author

I added support for form_args, and I also pass column labels into the form.

The implementation is... not the best. I still largely retain the behavior you have implemented, which is to loop over the str identifiers for the columns rather than the actual Columns. I think the "Form" part of the code is starting to outgrow mere strs. 😬 There should be some refactoring to make this work more with Column objects rather than strings, and a few other things. Hard to explain but I think you'll see what I mean when you peek at my code.

@dwreeves dwreeves force-pushed the main branch 2 times, most recently from 48e85bf to 0f8641d Compare March 21, 2022 02:31
@dwreeves
Copy link
Contributor Author

dwreeves commented Mar 21, 2022

Just added form_overrides as well.

Again, this will definitely need to be refactored in the future (I already implemented a little bit of refactoring of something that was there, i.e. the get_converter() method).

Regardless, it should be emulating the (high-level) behavior of the Flask-Admin API, and tests are in place and passing.

@aminalaee
Copy link
Owner

Hey, thanks a lot for this. It generally looks very good to me.
I wish you didn't add all of them in one PR as it makes the review a bit harder. But I'll try my best to review it today 👍

sqladmin/models.py Outdated Show resolved Hide resolved
sqladmin/models.py Outdated Show resolved Hide resolved
sqladmin/models.py Show resolved Hide resolved
sqladmin/models.py Outdated Show resolved Hide resolved
sqladmin/models.py Outdated Show resolved Hide resolved
@dwreeves
Copy link
Contributor Author

dwreeves commented Mar 21, 2022

✅ I removed most of the Optional[...]s from the class vars. I do think that form should remain optional (based on the specification outlined in the Flask-Admin API), but if you have an idea of an alternate implementation, let me know.

✅ I added coverage for the override of the scaffolding.

@dwreeves
Copy link
Contributor Author

dwreeves commented Mar 21, 2022

Woo, all checks passed! 🎉

Thanks for all the hard work you've put into this library so far. It was lots of fun to help contribute. This is a really good implementation of an admin panel framework.

I plan on making tons of use of this library for my own projects. You might see some more PRs from me in the near future, especially as it pertains to porting more things over from Flask-Admin.

Copy link
Owner

@aminalaee aminalaee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the great work @dwreeves 🎉

@aminalaee aminalaee merged commit 00ae267 into aminalaee:main Mar 21, 2022
@aminalaee
Copy link
Owner

Woo, all checks passed! 🎉

Thanks for all the hard work you've put into this library so far. It was lots of fun to help contribute. This is a really good implementation of an admin panel framework.

I plan on making tons of use of this library for my own projects. You might see some more PRs from me in the near future, especially as it pertains to porting more things over from Flask-Admin.

I really appreciate the effort and all sorts of contributions are welcome :)

I'm glad you'll use it and contribute to make it better.

@aminalaee aminalaee mentioned this pull request Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants