States per registration type / states as fields

Implementation would likely involve inventing bundles for registration_state entities, where bundle names are bundles of registration_type. Removing the default_state field from registration_state, and moving it to a new column of registration_type would also be required.

I think we would just need to add a registration_type col to the states schema, and default state would have to be unique across rows with the same registration type.

Re: fields, it's great to explore, but I'm still skeptical. E.g., how would we handle designating which states should be "active" and with that approach?

@dpi, thanks for your work on exploring this solution. I couldn't apply the patch cleanly, and received loads of errors after cleaning it up to apply enough to update, including calling non-existent functions and like. Taking it further, I was able to create a state, but there was no default state field applied to registration bundles. After I added one, I had two state fields appearing on the form.

Mind taking another pass at it so we can get a clear sense of what you're proposing?

@dpi, thanks this applied, although there are lots of spacing issues with the patch. Not saying that to be obstinate / anal (;), just much harder to review the patch.

I also don't see a state field being added when a new registration type is being created.

Specifically, what do you mean by spacing issues?

Indentation changed (incorrectly) in every modified file. E.g.,

function registration_field_extra_fields() {
  // expose the email property on the fields and display settings forms.
  $extra = array();
  foreach (registration_get_types() as $type => $reg_type) {
    $extra['registration'][$type] = array(
      'form' => array(
        'anon_mail' => array(
          'label' => t('Email'),
          'description' => t('Registrant\'s email address.'),
          'weight' => 0,
    ),
    ),
      'display' => array(
        'mail' => array(
          'label' => t('Email'),
          'description' => t('Registrant\'s email address.'),
          'weight' => 0,
    ),
        'host_entity_link' => array(
          'label' => t('Entity Link'),
          'description' => t('Link to host entity.'),
          'weight' => 0,
    ),
        'created' => array(
          'label' => t('Created'),
          'description' => t('When the registration was created.'),
          'weight' => 0,
    ),
        'updated' => array(
          'label' => t('Updated'),
          'description' => t('When the registration was updated.'),
          'weight' => 0,
    ),
        'slots' => array(
          'label' => t('Slots Used'),
          'description' => t('How many slots were used in this registration.'),
          'weight' => 0,
    ),
        'author' => array(
          'label' => t('Author'),
          'description' => t('User who created the registration.'),
          'weight' => 0,
    ),
        'user' => array(
          'label' => t('User'),
          'description' => t('User associated with this registration.'),
          'weight' => 0,
    ),
        'state' => array(
          'label' => t('State'),
          'description' => t('State of the registration.'),
          'weight' => 0,
    ),
    )
    );
  }

Thanks @dpi, applied clean and some great ideas in there. I see the benefits of the hybrid field approach, but still not sure. Below are some issues and thoughts.

• From my perspective, a registration needs to have a single, canonical, state. Otherwise, well, how do you know what state it's in? I don't think basing it on weight is sufficient.
• Similarly, we'd need to prevent adding more than one state field to registration bundles.
• I think there should at least be an option to automatically add state fields to registration bundles.
• Why the refactor of registration_states() to exclude a condition parameter? Seems less efficient and forces you to call EFQ directly in several places rather than using the API. Very minor point, but why also move them to a different file? Makes patch review harder ...
• registration_states_active() is testing for the hidden rather than active flag().
• What is registration_get_state_id() for?
• I think removing the active state filter from broadcast forms decreases utility for most use cases. The point is to email people who are registered for an event. If there state is, say, "canceled", we don't want to include them. I can see an additional form element allowing admins to select which states to include in the broadcase.

This sounds very promising, particularly with some of the broadcast changes you mention in the last update. What's the state (no pun intended) of review on this? I'll probably be testing this patch early next week as this will potentially cover some needs for a client of ours.

Thanks!

Jeremiah

Hi DPI,

I'm not having a lot of luck applying this patch. The patch doesn't apply to 7.x-1.x HEAD (of course). I was able to apply it with warnings to a checkout of 7.x-1.x from commit d720a0a4d29debb941831d77637d6de903f58f92 (May 14th from Brandon).

I then attempted to enable Registration on a clean Drupal install.

When attempting to enable Registration I get:

FieldException: Attempt to create a field of unknown type <em class="placeholder">registration_state</em>. in field_create_field() (line 110 of /www/managed/managed/drupal/releases/drupal-7/gose/drupal/modules/field/field.crud.inc).

This leaves me unable to create registration states or registration types. Creating a registration type results in:

ieldException: Attempt to create an instance of a field registration_state that doesn't exist or is currently inactive. in field_create_instance() (line 457 of /www/managed/managed/drupal/releases/drupal-7/gose/drupal/modules/field/field.crud.inc).

I'm really interested in seeing this move forward personally, your description above makes this sounds like a great direction for State handling in the module. I'd be interested to hear more from the maintainers about the direction and future of this. It'll probably be a bit of work by DPI or anyone else to bring this up to date with other changes that have been committed so it might be nice to hear more about the acceptance of the suggested changes?

Thanks for your work DPI, I'm really hoping this moves forward!

Jeremiah

Patch needs to be rerolled. It fails to apply.

i could not get it to apply cleanly to test either

Okay, I **MANUALLY** applied this patch to 7.x-1.x. It is now re-rolled, and the attached patch should now apply.

However, as I feared/expected, it still needs considerable work. For instance, when I enable the module now, I get:

FieldException: Attempt to create a field of unknown type <em class="placeholder">registration_state</em>. in field_create_field() (line 110 of /path/to/drupal/modules/field/field.crud.inc).

And tests can't be run. It gives a similar error.

My bad. I forgot a chunk of the code.

Attached is a better patch that not only applies, but actually works... mostly.

I'll keep this as "needs work" because 13 tests now fail. Also, tests should probably be written for the new functionality.

Checking in on this. More flexible registration would be a great thing to have, and while the new wait list functionality gets part way towards what we're looking for, it's not quite there. The project this is related to isn't completely ready to go - so we've been waiting and hoping these tools would mature more. At some point in the near future we'll probably be getting rolling though, and I'll need to decide how best to move forward.

That may include working on this patch, but I'd only want to invest that kind of time here if there's a high likelyhood of the work being accepted. All that to say, what are the maintainers feelings on this approach? I took Shawn's work on it in September as a positive sign, but hit hasn't been touched since and things like the waitlist and http://drupal.org/node/1871570 have been developed around this, rather than finishing this work first (likely making the implementation of this state handling rewrite more complex).

Just looking for some input here so I can judge how best to proceed when the time comes.

Thanks!

Jeremiah

@jerdavis, this is not hight our list nor have we heard much of outcry for adding this feature. I'm more inclined towards the states per type change than using fields for states at this point. So a patch addressing the former w/o the latter would certainly get serious considerations. Thanks for getting involved!

I am a big fan of this work, thinking that could be useful to have registration states for reach registration type, not same for every registration types on whole site.
Hope it will be working soon and it could be integrated into Registration core module.

Thanks for your work, guys

field per

If anyone is looking to have a default state per registration type, check out #2055145: Allow different default statuses for different registration types

As @ratface mentions - #2055145: Allow different default statuses for different registration types has been resolved, provides the ability to set a default state per registration type (if none is set, the global default registration state will be used), and will be available in the upcoming 7.x-1.4 release.

this sounds good! Looking forward to new release! :)

This is supported by the 3.x version of the module. Each registration type can have a different workflow.