Fix:1802 project summary bug #1815

BCerki · 2023-07-20T21:48:31Z

card: https://app.zenhub.com/workspaces/climate-action-secretariat-60ca4121764d710011481ca2/issues/gh/bcgov/cas-cif/1802

BCerki · 2023-07-24T20:20:39Z

schema/deploy/functions/handle_project_summary_report_form_change_commit.sql

+  reporting_requirement_record_id := fc.form_data_record_id;



This line was missing and that's what created the bug. This function returns reporting_requirement_record_id, so if we don't assign here, then the update and archive cases return null.

BCerki · 2023-07-24T20:25:24Z

schema/test/unit/mutations/create_project_revision_test.sql

      "projectId":1,
      "reportType": "General Milestone",
      "reportingRequirementIndex": 1
      }'::jsonb, 'milestone'::varchar),
-    ('{
+    (4,6,'{


If the commit handler returns null, then the form_data_record_id is also null. So when create_project_revision makes a copy of the existing forms, it just copies the original one over and over again instead of getting the latest one. Testing that form_data_record_id is correct here should ensure the bug is fixed

Sepehr-Sobhani

This looks great! However, I do have one concern. There are currently three projects in production that have an associated project summary report, but the values in the database are incorrect. To resolve this issue, I believe we should create a migration and commit the form changes related to those projects.
Will share Project IDs in the MS Teams channel.

Sepehr-Sobhani · 2023-07-26T21:19:01Z

schema/test/unit/mutations/create_project_revision_test.sql

+select '-----------';
+select form_data_record_id, previous_form_change_id, new_form_data, json_schema_name from cif.form_change
+    where form_data_table_name = 'reporting_requirement' and project_revision_id = 2
+    order by form_data_record_id;
+select '-----------';
+
+select '-----------';
+select form_data_record_id, previous_form_change_id, new_form_data
+  from cif.form_change
+      where form_data_record_id = 4
+      and form_data_schema_name = 'cif'
+      and form_data_table_name = 'reporting_requirement'
+      and json_schema_name='project_summary_report'
+      and change_status = 'committed'
+      order by updated_at desc, id desc;
+      -- limit 1;
+select '-----------';
+
+select '======';
+select id,
+  operation ,
+  form_data_schema_name,
+  form_data_table_name,
+  form_data_record_id ,
+  project_revision_id ,
+  change_status ,
+  json_schema_name,
+  validation_errors ,
+  previous_form_change_id from cif.form_change where json_schema_name='project_summary_report';
+select '======';
+
+


BCerki · 2023-07-31T17:33:22Z

This looks great! However, I do have one concern. There are currently three projects in production that have an associated project summary report, but the values in the database are incorrect. To resolve this issue, I believe we should create a migration and commit the form changes related to those projects. Will share Project IDs in the MS Teams channel.

I've written a migration to add the missing form_data_record_ids to the existing projects so that any new revisions of these projects will have the latest data. However, I think there might be a little manual reconstruction for the CIF team because the latest data isn't necessarily complete.

dleard

Looks good, just one suggestion to make and we need a migration to call the migration function. Make sure to disable these 3 triggers in the migration file:

_100_committed_changes_are_immutable
_set_previous_form_change_id,
_100_timestamps;

schema/deploy/functions/migration_rebuild_project_summary_report_history.sql

Sepehr-Sobhani

Looks good to me! There are only two minor changes needed.

Sepehr-Sobhani · 2023-08-08T15:52:51Z

schema/test/unit/functions/migration_rebuild_project_summary_report_history_test.sql

@@ -0,0 +1,85 @@
+
+begin;
+select no_plan();


schema/test/unit/functions/migration_rebuild_project_summary_report_history_test.sql

BCerki · 2023-08-08T22:26:30Z

I think I fixed the bug about the summary report summary not showing in view. I found another related one that wasn't straightforward so I made a card: https://app.zenhub.com/workspaces/climate-action-secretariat-60ca4121764d710011481ca2/issues/gh/bcgov/cas-cif/1844

Co-authored-by: Dylan Leard <[email protected]>

Sepehr-Sobhani

I did notice a few leftovers and have one small question. Can you please help me with that?
Otherwise, everything seems to be in order 🤩

Sepehr-Sobhani · 2023-08-09T20:23:05Z

app/components/Form/ProjectEmissionIntensityReportFormSummary.tsx

+  console.log("emissions");
+  console.log("allFormChangesPristine", allFormChangesPristine);
+  console.log("viewOnly", viewOnly);
+  console.log("summaryReportingRequirement", summaryReportingRequirement);


Sepehr-Sobhani · 2023-08-09T20:23:12Z

app/components/Form/ProjectEmissionIntensityReportFormSummary.tsx

@@ -223,7 +226,7 @@ const ProjectEmissionsIntensityReportFormSummary: React.FC<Props> = ({
        },
      },
    };
-
+  console.log("allFormChangesPristine", allFormChangesPristine);


Sepehr-Sobhani · 2023-08-09T20:23:23Z

app/components/Form/ProjectSummaryReportFormSummary.tsx

 }) => {
+  console.log("query", query);


Sepehr-Sobhani · 2023-08-09T20:23:28Z

app/components/Form/ProjectSummaryReportFormSummary.tsx

+    `,
+    query
+  );
+  console.log("projectSummaryReportFormBySlug", projectSummaryReportFormBySlug);


Sepehr-Sobhani · 2023-08-09T20:43:41Z

schema/test/unit/functions/migration_rebuild_project_summary_report_history_test.sql

+-- all updates have the same previous_form_change_id=1 and form_data_record_id=123 (bug=form_data_record_id clears on commit)
+select cif.create_form_change('update', 'project_summary_report', 'cif', 'project_summary_report', '{"description": "value"}', 123, 1, 'pending', '[]');
+
+select cif.create_form_change('update', 'project_summary_report', 'cif', 'project_summary_report', '{"description": "most recent"}', 123, 1, 'pending', '[]');
+
+select cif.commit_project_revision(1);


Now that you've resolved the problem with the summary report handler, I'm wondering if we can be certain that the commit handler will clear the form_data_record_id upon commit. Do you think it would be better to simply pass null values for form_data_record_id instead?

Great point, thanks for catching this!

Sepehr-Sobhani

Awesome 🤩

Sepehr-Sobhani · 2023-08-10T18:37:45Z

schema/test/unit/functions/migration_rebuild_project_summary_report_history_test.sql

-select '-----';
-select * from cif.form_change order by id asc;
-select '-----';
-select cif_private.migration_rebuild_project_summary_report_history();


I think we should still keep this line! sorry for the hassle.

Addressed.

BCerki force-pushed the 1802-project-summary-bug branch 3 times, most recently from 70b12df to 85d1cc2 Compare July 24, 2023 20:18

BCerki commented Jul 24, 2023

View reviewed changes

BCerki force-pushed the 1802-project-summary-bug branch from 85d1cc2 to 8451e31 Compare July 24, 2023 20:23

BCerki commented Jul 24, 2023

View reviewed changes

BCerki force-pushed the 1802-project-summary-bug branch from 8451e31 to a2973f6 Compare July 25, 2023 14:21

Sepehr-Sobhani reviewed Jul 26, 2023

View reviewed changes

BCerki force-pushed the 1802-project-summary-bug branch 2 times, most recently from 47cc522 to 7970d7f Compare July 31, 2023 17:26

dleard previously requested changes Aug 4, 2023

View reviewed changes

schema/deploy/functions/migration_rebuild_project_summary_report_history.sql Outdated Show resolved Hide resolved

BCerki force-pushed the 1802-project-summary-bug branch from 2cf3e7a to 34db13c Compare August 8, 2023 15:17

Sepehr-Sobhani requested changes Aug 8, 2023

View reviewed changes

BCerki and others added 9 commits August 9, 2023 11:58

test: pgtap test for missing forms

e9b0ade

fix: project summary report correctly updates

20a2959

chore: apply suggestions from code review

578a78c

chore: migration to fix missing project summary form_data_record_id

5128a55

chore: apply suggestions from code review

65f61fe

Co-authored-by: Dylan Leard <[email protected]>

chore: apply suggestions from code review

81f1343

chore: apply suggestions from code review

272b800

chore: fix hidden summary

2afce18

chore: fix CI

00246c4

BCerki force-pushed the 1802-project-summary-bug branch from 764421c to 00246c4 Compare August 9, 2023 18:59

Sepehr-Sobhani requested changes Aug 9, 2023

View reviewed changes

BCerki added 3 commits August 10, 2023 09:23

chore: apply suggestions from code review

529079d

chore: apply suggestions from code review

9541a2d

chore: apply suggestions from code review

04c3f10

Sepehr-Sobhani approved these changes Aug 10, 2023

View reviewed changes

BCerki merged commit 55379bd into develop Aug 10, 2023
15 of 16 checks passed

BCerki deleted the 1802-project-summary-bug branch August 10, 2023 20:24

bc-cas-shipit bot requested a deployment to dev August 10, 2023 20:50 Pending

bc-cas-shipit bot requested a deployment to test August 16, 2023 16:27 Pending

bc-cas-shipit bot requested a deployment to prod August 24, 2023 16:21 Pending

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix:1802 project summary bug #1815

Fix:1802 project summary bug #1815

BCerki commented Jul 20, 2023

BCerki Jul 24, 2023

BCerki Jul 24, 2023 •

edited

Loading

Sepehr-Sobhani left a comment •

edited

Loading

Sepehr-Sobhani Jul 26, 2023

BCerki commented Jul 31, 2023

dleard left a comment

Sepehr-Sobhani left a comment

Sepehr-Sobhani Aug 8, 2023

BCerki commented Aug 8, 2023

Sepehr-Sobhani left a comment

Sepehr-Sobhani Aug 9, 2023

Sepehr-Sobhani Aug 9, 2023

Sepehr-Sobhani Aug 9, 2023

Sepehr-Sobhani Aug 9, 2023

Sepehr-Sobhani Aug 9, 2023

BCerki Aug 10, 2023

Sepehr-Sobhani left a comment

Sepehr-Sobhani Aug 10, 2023

Fix:1802 project summary bug #1815

Fix:1802 project summary bug #1815

Conversation

BCerki commented Jul 20, 2023

Choose a reason for hiding this comment

BCerki Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Sepehr-Sobhani left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BCerki commented Jul 31, 2023

dleard left a comment

Choose a reason for hiding this comment

Sepehr-Sobhani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BCerki commented Aug 8, 2023

Sepehr-Sobhani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sepehr-Sobhani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BCerki Jul 24, 2023 •

edited

Loading

Sepehr-Sobhani left a comment •

edited

Loading