DTAMM-31: Add Event Types Filters To Event Certificates #114
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds event type filtering capability to event certificates, bringing consistency with membership and case certificate configurations. Previously, event certificates could only be configured for specific events, while membership and case certificates supported filtering by type.
Key Changes:
- Added multi-select Event Types field to event certificate configuration UI with validation
- Stored event type selections in a new database column alongside participant roles
- Updated certificate retrieval logic to enforce event type matching during download
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| xml/schema/CRM/Certificate/CompuCertificateEventAttribute.xml | Added event_type_ids column definition to schema |
| sql/upgrade_0004.sql | Migration script to add event_type_ids column |
| sql/auto_install.sql | Updated auto-install schema with event_type_ids column |
| CRM/Certificate/Upgrader.php | Added upgrade_0004 method to execute migration |
| CRM/Certificate/Form/CertificateConfigure.php | Added event_type_ids field, validation, and notice messaging |
| templates/CRM/Certificate/Form/CertificateConfigure.tpl | Added UI logic for event types field visibility and participant type enabling |
| templates/CRM/Certificate/Form/CertificateConfigure.hlp | Added help text for event types field |
| CRM/Certificate/Service/CertificateEvent.php | Implemented event type condition logic for duplicate detection |
| CRM/Certificate/BAO/CompuCertificateEventAttribute.php | Updated to store and sanitize event type IDs |
| CRM/Certificate/DAO/CompuCertificateEventAttribute.php | Auto-generated DAO with event_type_ids property |
| CRM/Certificate/Entity/Event.php | Added event type filtering to certificate retrieval and configuration display |
| CRM/Certificate/Entity/AbstractEntity.php | Updated getCertificateConfiguredTypes signature with includeAttrs parameter |
| CRM/Certificate/Entity/Membership.php | Updated getCertificateConfiguredTypes signature for consistency |
| CRM/Certificate/Entity/Case.php | Updated getCertificateConfiguredTypes signature for consistency |
| CRM/Certificate/Page/ConfigureCertificate.php | Updated call to getCertificateConfiguredTypes with includeAttrs flag |
| tests/phpunit/CRM/Certificate/Service/CertificateEventTest.php | Added test for event type IDs storage |
| tests/phpunit/CRM/Certificate/Entity/EventsTest.php | Added tests for event type filtering and combined event/type requirements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
shahrukh-compuco
force-pushed
the
dtamm-31-event-types
branch
2 times, most recently
from
fef443d to
9e4cf25
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
shahrukh-compuco
force-pushed
the
dtamm-31-event-types
branch
from
9e4cf25 to
1b64f46
Compare
erawat
left a comment
erawat
left a comment
There was a problem hiding this comment.
@shahrukh-compuco the Before image as Event Types.
CRM/Certificate/Entity/Event.php
Outdated
| 'is_active' => 1, | ||
| ]); | ||
| $participantRoleIds = implode(',', (array) $participant['participant_role_id']); | ||
| $event = civicrm_api3('Event', 'getsingle', [ |
There was a problem hiding this comment.
@shahrukh-compuco can we use API4? I think we should obmit from using getsingle API as it will throws error if the record does not exist.
shahrukh-compuco
force-pushed
the
dtamm-31-event-types
branch
2 times, most recently
from
5b3a05f to
1b64f46
Compare
shahrukh-compuco
force-pushed
the
dtamm-31-event-types
branch
from
1b64f46 to
04cbddb
Compare
Overview
Currently, there is some inconsistency on the Certificates configuration UI which is that in both membership and cases certificates can be configured for membership and event type respectively, but for events we only allow configuration for a specific event and not the event types, so this PR
Before
After
Technical Details
We add a new column to compucertificate_event_attribute table to store multiple comma separated event type ids for a certificate when user creates or updates a certificate configuration. Currently we allow setting up event types and specific event as well while configuring the certificate but we show a notice to the user regarding selection of both the settings. When a user is trying to download the certificate we determines the availability of certificate based on these rules
If only an event was selected then only issue if the configured event matches the current event.
If only an event type was selected then only issue if the current event belongs to one of configured event types.
If both an event type and and event were selected then only issue if BOTH match.