Fix release field missing on admin interface #316

Merged
jgroman merged 4 commits from fix/279-release-input into develop 2026-06-22 13:42:31 +00:00
Owner

Restores release field in admin interface form

  • Force SQLAlcheny relationships init before rendering the form
  • Add tests covering this problem

Fixes 279

Assisted-by: Claude Code

Restores release field in admin interface form - Force SQLAlcheny relationships init before rendering the form - Add tests covering this problem Fixes 279 Assisted-by: Claude Code
jgroman self-assigned this 2026-06-08 12:52:22 +00:00
- Force SQLAlcheny relationships init before rendering the form
- Add tests covering this problem

Fixes 279

Assisted-by: Claude Code
Remove redundant test
Some checks failed
Run tests and linters / test (pull_request) Successful in 2m48s
Run tests and linters / lint (pull_request) Failing after 2s
AI Code Review / ai-review (pull_request_target) Successful in 13s
926b60ab01
Author
Owner

image

![image](/attachments/b929fadb-119b-43a5-9f84-2f7a7871b7f0)
jgroman changed title from WIP: Fix release field missing on admin interface to Fix release field missing on admin interface 2026-06-09 09:52:33 +00:00

AI Code Review

📋 MR Summary

Fixes a bug where the release field was missing from the admin interface by forcing SQLAlchemy relationship initialization.

  • Key Changes:
    • Added configure_mappers() call before Flask-Admin view registration to ensure relationships are initialized.
    • Added new test suite TestMilestoneAdminForm and TestReleaseAdminForm to verify admin form fields.
  • Impact: blockerbugs/controllers/admin/init.py, testing/unit/test_admin.py
  • Risk Level: 🟢 Low - The fix is a standard SQLAlchemy lifecycle adjustment with low risk of negative side effects, well isolated to admin setup.

Detailed Code Review

The solution properly addresses the missing field issue caused by deferred relationship initialization in SQLAlchemy 2.0. The placement of configure_mappers() is appropriate. The added tests are excellent for preventing future regressions.

Summary

  • Overall Assessment: The changes are solid and correctly resolve the missing field issue while adding robust regression testing.

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### 📋 MR Summary Fixes a bug where the `release` field was missing from the admin interface by forcing SQLAlchemy relationship initialization. - **Key Changes:** - Added `configure_mappers()` call before Flask-Admin view registration to ensure relationships are initialized. - Added new test suite `TestMilestoneAdminForm` and `TestReleaseAdminForm` to verify admin form fields. - **Impact:** blockerbugs/controllers/admin/__init__.py, testing/unit/test_admin.py - **Risk Level:** 🟢 Low - The fix is a standard SQLAlchemy lifecycle adjustment with low risk of negative side effects, well isolated to admin setup. ### Detailed Code Review The solution properly addresses the missing field issue caused by deferred relationship initialization in SQLAlchemy 2.0. The placement of `configure_mappers()` is appropriate. The added tests are excellent for preventing future regressions. ### ✅ Summary - **Overall Assessment:** The changes are solid and correctly resolve the missing field issue while adding robust regression testing. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*
jgroman requested review from kparal 2026-06-09 09:54:22 +00:00
Author
Owner

Ready for review

Ready for review
Owner

There's a lot of repetition in the tests. Can we factor that out? I like using pytest parametrize, but however you want to do it...it'd just be nice not to repeat 99% of the code.

There's a lot of repetition in the tests. Can we factor that out? I like using pytest parametrize, but however you want to do it...it'd just be nice not to repeat 99% of the code.
kparal left a comment

The initial commit misses # on the Fixes line. Please correct that in the final commit, thanks!

The initial commit misses `#` on the `Fixes` line. Please correct that in the final commit, thanks!
@ -92,2 +93,4 @@
# Force SQLAlchemy to initialize relationship metadata before Flask-Admin introspects
# models to scaffold forms.
Owner

For those of us not so strong in SQLAlchemy, can this also explain why? 🙂

For those of us not so strong in SQLAlchemy, can this also explain _why_? 🙂️
Author
Owner

Expanded the comment.

Expanded the comment.
jgroman marked this conversation as resolved
Owner

I tested the code and it works. Surprisingly it adds more fields to the Milestone and Release forms, like Bugs, Updates and Succeeds. I don't know if they were a part of it originally or not, but it doesn't matter, we don't need to touch them. Most importantly, new releases and milestones can now be added directly from the admin UI.

I tested the code and it works. Surprisingly it adds more fields to the Milestone and Release forms, like Bugs, Updates and Succeeds. I don't know if they were a part of it originally or not, but it doesn't matter, we don't need to touch them. Most importantly, new releases and milestones can now be added directly from the admin UI.
Owner

@adamwill wrote in #316 (comment):

There's a lot of repetition in the tests. Can we factor that out? I like using pytest parametrize, but however you want to do it...it'd just be nice not to repeat 99% of the code.

I don't have a strong opinion on this. I myself repeat code a lot in tests. While it can be often abstracted, it then hurts readability. Depends on a particular case. Here it's just two tests, not ten, so I don't really mind.

@adamwill wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/316#issuecomment-819174: > There's a lot of repetition in the tests. Can we factor that out? I like using pytest parametrize, but however you want to do it...it'd just be nice not to repeat 99% of the code. I don't have a strong opinion on this. I myself repeat code a lot in tests. While it can be often abstracted, it then hurts readability. Depends on a particular case. Here it's just two tests, not ten, so I don't really mind.
Fix review findings: expand comment, rework tests
Some checks failed
Run tests and linters / test (pull_request) Successful in 2m50s
Run tests and linters / lint (pull_request) Failing after 2s
c73654e934
Author
Owner

Ready for review

Ready for review
jgroman force-pushed fix/279-release-input from c73654e934
Some checks failed
Run tests and linters / test (pull_request) Successful in 2m50s
Run tests and linters / lint (pull_request) Failing after 2s
to d4beae03cb
Some checks failed
Run tests and linters / test (pull_request) Failing after 2m44s
2026-06-22 13:24:55 +00:00
Compare
kparal approved these changes 2026-06-22 13:30:56 +00:00
Reformat test_admin.py for format check
All checks were successful
Run tests and linters / test (pull_request) Successful in 2m40s
9392572d0d
jgroman merged commit 9392572d0d into develop 2026-06-22 13:42:31 +00:00
jgroman deleted branch fix/279-release-input 2026-06-22 13:42:31 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
quality/blockerbugs!316
No description provided.