-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/345 load activity data from database #2446
Conversation
33470c9
to
6b3b0b9
Compare
ddf1497
to
ea8e9f6
Compare
ea8e9f6
to
c508d59
Compare
import { withTheme } from "@rjsf/core"; | ||
import formTheme from "@bciers/components/form/theme/defaultTheme"; | ||
|
||
const Form = withTheme(formTheme); |
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.
FormBase
was introducing some unintended behaviours by also holding a copy of the formData in the state, so this uses the RJSF component directly
622aa16
to
773ed47
Compare
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.
Looks great! Love how those serializers work. Just one note on a potential performance optimization.
I've made a bug card for the issue this PR unearthed around the persistence of formState that should be handled shortly after this goes in
def load(cls, report_version_id: int, facility_id: UUID, activity_id: int) -> dict: | ||
|
||
try: | ||
r = ReportActivity.objects.get( |
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.
Could this query benefit from prefetch_related?
2351deb
to
1676986
Compare
ReportActivity
,ReportUnit
,ReportFuel
,ReportEmission
andReportMethodology
in a format that the activity form can display and understandActivityForm
to update the form data with the saved IDs and computed data