Contrib.social

Provide a default value for the registration type

Open on Drupal.org →
Created on 19 June 2024, 8 months ago

Problem/Motivation

When event registrations are enabled, there are 2 possible values for the registration type:

  1. series: The registration is for the entire event series
  2. instance: The registration is only for a single event instance

We are defaulting to instance in the field widget (ref EventRegistrationWidget::formElement()), but outside of this widget there is no default provided. When event series are created programmatically, or when the Recurring Events Registration module is enabled after event series have already been created the registration type can be missing. There is also no check in place that the value is actually correct.

If the registration type is not present this can cause weird issues since our code relies on it being available. Examples:

  • RegistrationCreationService::retrieveRegisteredPartiesCount() will return the full count of all registrants across all events.
  • RegistrationCreationService::retrieveRegisteredParties() returns the entire registrant dataset instead of just the registrants for the event.
  • RegistrationCreationService::registrationOpeningClosingTime() throws a fatal error Call to member function setTimeZone() on NULL.
  • RegistrationCreationService::promoteFromWaitlist() silently skips promoting a registrant from the waitlist, leaving an empty spot.

Steps to reproduce

\Drupal\recurring_events\Entity\EventSeries::create(['type' => 'default'])->event_registration->registration_type

Expected result: a valid default value such as instance
Actual result: NULL

Proposed resolution

  • We should first of all provide a default value for the property in EventRegistration::propertyDefinitions().
  • Since this can only be one of two fixed values, it would be really nice if we would encode the values as an enum.
    declare(strict_types=1);

namespace Drupal\recurring_events_registration\Enum;

/**
 * Enumerates the available registration types.
 */
enum RegistrationType: string {
  case INSTANCE = 'instance';
  case SERIES = 'series';

  /**
   * Returns the default value.
   */
  public static function defaultValue(): self {
    return self::INSTANCE;
  }

}
  • Wherever we are accessing the registration type, let's make sure to fall back to the default value. For example let's refactor RegistrationCreationService::getRegistrationType(). Note also that the method documentation shows that the intention was to always return a value, and not FALSE as we are doing now.

    Before:

      /**
   * Get registration type.
   *
   * @return string
   *   The type of registration: series, or instance.
   */
  public function getRegistrationType() {
    $type = FALSE;

    if (!empty($this->eventSeries->event_registration->registration_type)) {
      $type = $this->eventSeries->event_registration->registration_type;
    }

    return $type;
  }

    After:

      /**
   * Returns the registration type.
   *
   * @return string
   *   The type of registration: series, or instance.
   *   Starting from Recurring Events 3.x this will return an instance of the
   *   RegistrationType enum.
   */
  #[\ReturnTypeWillChange]
  public function getRegistrationType(): string {
    $type = RegistrationType::tryFrom($this->eventSeries->event_registration->registration_type) ?? RegistrationType::defaultValue();
    return $type->value;
  }
  • Clean up the few instances where we are actually handling the missing value, for example in EventSeriesRegistrationType::render():
        $registration_type = $this->registrationCreationService->getRegistrationType();
    if (empty($registration_type)) {
      return $this->t('N/A');
    }
✨ Feature request
Status

Active

Version

2.0

Component

Recurring Events Registration (Submodule)

Created by

🇧🇬Bulgaria pfrenssen Sofia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @pfrenssen
  • Merge request !96Provide a default value for the registration type. → (Merged) created by pfrenssen
  • Pipeline finished with Failed
    8 months ago
    Total: 178s
    #202973
  • Status changed to Needs review 8 months ago
  • Comment 8 months ago →
    🇧🇬Bulgaria pfrenssen Sofia

    Posted a proposed fix in the MR. Setting the default value on the field property does not seem to take effect for string based properties so I skipped that part. Tests are failing due to 🐛 Failing tests on main branch Active .

  • Comment 8 months ago →
    🇧🇬Bulgaria pfrenssen Sofia

    Great idea to open a 3.x branch! We can set Drupal 10 as a minimum which will allow us to safely use PHP 8 features without breaking B/C. D9 is already EOL but there are possibly still some users who didn't have time to update yet.

  • Pipeline finished with Success
    8 months ago
    Total: 152s
    #204886
  • Comment 8 months ago →
    🇧🇬Bulgaria pfrenssen Sofia

    Updated the code to remove the B/C layer that was converting the enum back to string based registration types. This is a pure search/replace job, I have not tried the code.

  • Pipeline finished with Success
    8 months ago
    Total: 177s
    #205003
  • Pipeline finished with Success
    8 months ago
    Total: 179s
    #205013
  • Pipeline finished with Success
    7 months ago
    Total: 206s
    #206864
  • Comment 4 months ago →
    🇧🇬Bulgaria pfrenssen Sofia
  • Comment 4 months ago →
    System Message
    • pfrenssen → committed 2926dd41 on 3.0.x 
      Issue #3455716 by pfrenssen: Provide a default value for the...
  • Comment 4 months ago →
    🇧🇬Bulgaria pfrenssen Sofia

    I'm going to go ahead and merge this in 3.x. We've been using this successfully in production for several months, and 3.x is newly opened and doesn't have any releases yet, so this shouldn't affect any users.

  • Comment 4 months ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Comment 4 months ago →
    🇧🇬Bulgaria pfrenssen Sofia

    Looks like one instance was missed, in \Drupal\recurring_events_registration\RegistrationCreationService::registrationOpeningClosingTime():

            case 'scheduled':
          switch ($reg_type) {
            case RegistrationType::SERIES:
              // ...
              break;

            case 'instance': // <<< Needs to be replaced with the enum value
              // ...
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024