Skip to content

Conversation

@vahidbazzaz
Copy link
Contributor

This PR adds comprehensive history logging for family merge and detach operations in the beneficiary management system. Previously, when families were merged or detached (either via action buttons or drag & drop), no audit trail was created in the history table. This made it impossible to investigate cases where beneficiaries (including small children) were unexpectedly deactivated or moved between families.

Changes Made

1. In Manage Beneficiaries add history log if beneficiary is added to a family via drag & drop

  • File: include/people.php:503
  • Added history logging when beneficiary is added to a family via drag & drop
  • Log format: added to family via drag & drop

2. In Manage Beneficiaries add history log if beneficiary is removed from a family via drag & drop

  • File: include/people.php:506
  • Added history logging when beneficiary is removed from a family via drag & drop
  • Log format: removed from family via drag & drop

3. In Manage Beneficiaries add history log if beneficiaries are merged to a family via selection and action button

  • File: include/people.php:442
  • Added history logging when beneficiaries are merged into a family via action button
  • Log format: merged to family (head: [id])

4. In Manage Beneficiaries add history log if beneficiaries are detached from a family via selection and action button

  • File: include/people.php:468
  • Added history logging when beneficiaries are detached from a family via action button
  • Log format: detached from family

5. Drag & Drop Implementation Details

  • Files: library/lib/list.php:58-94
  • Modified listBulkMove() to track parent_id changes during drag & drop operations
  • Returns parent change information to enable history logging

6. Cypress Tests

  • Files: cypress/support/database.js, cypress/e2e/1_feature_tests/5_3_Manage_Beneficiaries.js
  • Added checkHistoryLog() command to verify history entries exist in database
  • Added getBeneficiaryIdFromRow() command to extract beneficiary IDs from DOM
  • Created test helper endpoint library/ajax/testhistorycheck.php for test verification
  • Enhanced existing merge/detach tests to verify history logging works correctly

Testing

✅ All Cypress tests passing:

  • Merge beneficiaries: Verifies history log created
  • Detach beneficiaries: Verifies history log created
  • Existing tests: All still passing

@vahidbazzaz
Copy link
Contributor Author

@vahidbazzaz vahidbazzaz requested a review from HaGuesto November 20, 2025 15:25
Copy link
Member

@HaGuesto HaGuesto left a comment

Choose a reason for hiding this comment

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

@Vahid I tested this a bit more in depth and would say we need another iteration:

from the functional test:

  1. the lastModified date in the people table is somehow not updated could you maybe adapt the definition of the simpleBulkSaveChangeHistory to do this too?

from the code review

  • since we do not show the history messages anywhere, I'm happy how they look like atm, but it would be great if you could move the from and to values out of the change message into the right bits of simpleBulkSaveChangeHistory($table, $records, $changes, $from = [], $to = [])
  • maybe put "parent_id;" in front in the change message to make clear that the parent_id in the record was changed.

I left comments in the code to make it clearer

}
}
});
simpleBulkSaveChangeHistory('people', $ids, 'merged to family (head: '.$oldest.')');
Copy link
Member

Choose a reason for hiding this comment

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

this should maybe be simpleBulkSaveChangeHistory('people', $ids, 'parent_id; merged to family', [], ['int'=> <parent_id>);

db_query('UPDATE people SET parent_id = NULL WHERE id = :id', ['id' => $id]);
}
});
simpleBulkSaveChangeHistory('people', $ids, 'detached from family');
Copy link
Member

Choose a reason for hiding this comment

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

this should maybe be simpleBulkSaveChangeHistory('people', $ids, 'parent_id; detached from family', ['int'=> <parent_id>, []);

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.

3 participants