I was reading the (ancient) feature requests at #91686: Improvement of scheduling workflow because I was looking for a way to add repeating schedule items. I quickly realized that adding and especially managing repeating items would require a lot of code, which probably isn't worth the trouble. However it would make entering schedule items easier if you had the possibility to duplicate an item. This patch allows for that by adding a single checkbox to the form. Patch (against beta3) and screenshot attached.

CommentFileSizeAuthor
#6 station-698832-6.patch1.82 KBtim.plunkett
#4 station-698832-4.patch2.07 KBtim.plunkett
station-schedule-item-duplicate.png26.17 KBmarcvangend
station_schedule_duplication.patch1.55 KBmarcvangend

Comments

drewish’s picture

drewish commented

I like the idea but would it be better to have it as an additional duplicate button? they could click that rather than having the check box?

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented

I considered that too and I'm still not sure what users would expect and/or prefer. In the end I chose the checkbox because it allows for a description, so I had more space to explain what it does. On the other hand, a good button label would not need a description anyway.

Coming to think of it, I'm not sure if the word "duplicate" is right here, because you're not creating an exact duplicate of course. Maybe something like "new" or "copy" would be more appropriate.

I'd like to have some more votes on this (I'll ask my colleagues tomorrow what they think), but how about a "Save as new" button? ASCII mockup:

[Save]  [Save as new]  [Remove]  [Cancel]
refreshingapathy’s picture

refreshingapathy commented

I'd vote for the "Save as New" button - slightly less confusing than the checkbox in my opinion.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 6.x-2.0-beta3 » 6.x-2.0-beta4
StatusFileSize
new station-698832-4.patch2.07 KB

How's this look?

As someone who has to make a new schedule every 3 months, I love this functionality.

drewish’s picture

drewish commented
Status: Needs review » Needs work

That seems fine but couldn't station_schedule_item_edit_duplicate_submit() just check the submit op? I guess this is probably cleaner since you'd don't have to worry about a t()'d string.

Oh and 

'iid' => $form_state['duplicate'] ? NULL : $form_state['values']['iid'],

will cause a PHP notice for not having $form_state['duplicate'] defined when you submit using the save handler.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 6.x-2.0-beta4 » 6.x-2.x-dev
Status: Needs work » Needs review
StatusFileSize
new station-698832-6.patch1.82 KB

I was trying to avoid checking a t()'d string, because I'm still not sure how that works. Is it as simple as ($form_state['values']['op'] == t('Save as new'))?

My other idea was to use $form_state['clicked_button']['#id'], which also gets rid of the PHP notice. That's the attached patch.

drewish’s picture

drewish commented

That looks better but I think you could simplify it further:

function station_schedule_item_edit_form_submit($form, &$form_state) {
  if ($form_state['clicked_button']['#id'] == 'edit-duplicate') {
    $form_state['values']['iid'] = NULL;
  }
  $program = node_load(array('type' => 'station_program', 'title' => $form_state['values']['program_title']));
  $schedule = node_load($form_state['values']['schedule_nid']);
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work

In all of these patches, station_schedule_item_edit_form_validate() breaks with a null iid, it doesn't catch overlaps. I'll have to take a look at this tomorrow.

drewish’s picture

drewish commented

Humm, currently it's doing:

  $result = db_query('SELECT count(*) AS count, min(s.start) AS start, max(s.finish) AS finish FROM {station_schedule_item} s WHERE s.schedule_nid = %d AND s.iid <> %d AND s.finish > %d AND s.start < %d', $form_state['values']['schedule_nid'], $form_state['values']['iid'], $start, $finish);

If iid is null it should what be cast to 0?