Problem/Motivation

Working on #3464736: Replace HostEntity::isEnabledForRegistration it was identified that it is extremely hard to understand how the waitlist validation event subscriber handles existing registrations correctly, and that there are probably edge cases it does not handle properly.

Possibly #3497722: Waitlist validation subscriber better as swapped plugin class might help simplify the logic here.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork registration-3497727

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathanshaw created an issue. See original summary.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Active » Postponed
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Postponed » Active

jonathanshaw opened merge request !85

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Active » Needs review

Studying this I've arrived at a conclusion:
(1) The current subscriber may be significantly broken, as shown by the failing test I've added.
(2) If we fix hasRoom() so it obeys its interface and returns a truthful answer instead of always returning true, then all the subscriber needs to do is to enhance the message/code if the waitlist is full.
(3) If the subscribers function is simple like this, then it makes sense as a subscriber and I don't see a particular need for #3497722: Waitlist validation subscriber better as swapped plugin class.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Needs review » Needs work

Unexpected test fail, needs more work.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Needs work » Needs review

john.oltman made their first commit to this issue’s fork.

john.oltman’s picture

john.oltman commented
Title: Refactor registration_waitlist RegistrationValidationEventSubscriber » Refactor validation result subscriber in the waitlist submodule
Issue summary: View changes
Status: Needs review » Fixed
Related issues: -#3497722: Waitlist validation subscriber better as swapped plugin class

This is a nice improvement, thanks @jonathanshaw.

There is a failure in phpcs that can be ignored re: use statements in the Drush command, it is due to a regression upstream in the coder contrib module, and the maintainers there are working to fix it.

For now, committing only to 3.3.x, pending discussion in #3497738: Roadmap for 3.4 release around the future of the 3.1.x branch.

Status: Fixed » Closed (fixed)

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