Using Entity Registration with the commerce module and have enabled the 3 states 'Pending, Completed and Cancelled'. A record is created in the registration table during the checkout process with the default state 'Pending' but if the user should cancel out of the checkout process during the payment/review step and then start the checkout process again then when they step thru the checkout process again, when they get to the 'Registration" Step, they don't see the custom fields (form) in the register checkout pane, they only see 'You do not have permission to register for this item'

In the function commerce_registration_information_checkout_form(), there is a call to registration_register_page_access() which calls registration_is_registered() and this is the source of the issue, it only checks if a registration record exists and does not check the record status.

Adding the following change to the registration module is my suggested fix but would like to hear other comments or ideas.

Current Code

$query = db_select('registration', 'r')
  ->condition('entity_id', $registration->entity_id)
  ->condition('entity_type', $registration->entity_type);

Changed code -- added test for default_state and added query condition.

$default_state = registration_get_default_state();

$query = db_select('registration', 'r')
  ->condition('entity_id', $registration->entity_id)
  ->condition('entity_type', $registration->entity_type)
  ->condition('state', $default_state->registration_state_id, '<>');
CommentFileSizeAuthor
#6 1881212.diff634 bytesdrumm
#2 1881212.diff760 bytesdrumm

Comments

drumm’s picture

Issue tags: +portland2013

I think we will run into this on DrupalCon sites.

drumm’s picture

Project: Commerce Registration » Entity Registration
Version: 7.x-3.x-dev » 7.x-1.x-dev
Component: Checkout » Registration Core
Assigned: Unassigned » drumm
Status: Active » Needs review
StatusFileSize
new760 bytes

Yes, we did run into this.

The attached patch adds a condition to only look for completed registrations, if there is a "complete" state.

I'm not sure if it is ideal since it removes the ability to check if someone has a registration in another state and hard-codes special behavior for a state name. I welcome any feedback to get this resolved.

drumm’s picture

Title: You do not have permission to register for this item » registration_is_registered() should be smart about registration states
blainelang’s picture

Neil, if for some reason the site was not using the 'Completed' state then why not use the condition that I had "anything other then default" - The state would have to be changed by rules or some other custom code during the checkout (most likely upon payment complete).

My thinking was that as long as it's still the default state then we want to allow the registration checkout pane to still be used - does it matter if it's state is 'Completed' or 'Being Screened" ?

drumm’s picture

Status: Needs review » Needs work

One of the default states is "Cancelled", which shouldn't be considered as registered.

I'm looking into using

$query->condition('state', registration_get_active_states(), 'IN');

This initially looks okay since it is used elsewhere in the registration module.

drumm’s picture

Status: Needs work » Needs review
StatusFileSize
new634 bytes

I feel better about this patch, it uses the active states, like registration_event_count() and registration_send_broadcast(). Commerce registration does not have any special treatment of actie states that I can see, so it should be okay.

tauno’s picture

Status: Needs review » Needs work

Using registration_get_active_states makes sense with commerce_registration, but registration_is_registered() is also used in 'registration_access_people' to determine if "Myself" is an option during registration. In that usage we don't care about active since we want to prevent a duplicate registration for the same entity with the same user/anon_mail properties (IF this limitation makes sense is another question, but there is an existing unique index on 'entity_id', 'entity_type', 'anon_mail', and 'user_uid'). registration_is_registered is also used in tests, so we need to check how it's being used there and update as necessary.

Maybe an active bool parameter defaulting to the current behavior of not using a state condition or to be even more flexible an array of states as a parameter. registration_get_active_states could be used to check active registrations, or it could be used to check other states as well.

drumm’s picture

For registration_access_people() we do want people to be able to register again if they have an inactive registration. If they abandoned or canceled a previous registration, they should be able to start again. The active registration type gives admins the flexibility to decides what counts or not, blainelang would leave "Being Screened" as active.

Adding the bool parameter, using it in registration_access_people(), and updating the index makes sense to me. Is that a good way to move forward?

tauno’s picture

#1861364: User can't register more than once? is a similar discussion about allowing multiple registrations by the same anon_mail or user.

Would it make more sense for commerce_registration to implement its own check that re-uses the existing pending registration instead of creating a fresh one?

drumm’s picture

Commerce registration tries to use as much as it can from registration's form, including

$who_options = registration_access_people($registration);

If the user has a registration with an inactive status, that still removes Myself from the options. I think this issue is now about allowing someone to register when they have a registration with an inactive status. This seems more consistent with registration_event_count() and registration_send_broadcast(); registrations with an inactive status don't count as registrations.

A couple alternatives to #8:

  • These registrations could be deleted, either by registration module or commerce registration, but that seems a bit sloppy. And I'm not sure there is a good place to do that.
  • Registration could notice this situation and keep Myself as an option, updating the existing registration on save.
drumm’s picture

On #1861364: User can't register more than once?, we still want to limit registrations to one (active) per person.

jhm’s picture

patch #6 works for me

levelos’s picture

Thanks @drumm et al. Commit 9c8456f64478a98afab7929609c34ee6f590ebdf contains a slight variation of #6 adding a bit more flexibility. We also remove the unique constraint discussed in #1861364: User can't register more than once?.

levelos’s picture

Status: Needs work » Fixed

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

  • levelos committed 9c8456f on 7.x-1.x, panels, any-entity, slots, integrations, hold_state
    #1881212: Alter registration_is_registered() to take registration states...