There is an extraneous/erroneous chunk of step removal code still in media_mover_configuration::remove(), attached patch removes it and makes a slight update to the delete message to include configuration name/description.

CommentFileSizeAuthor
config_delete_fix.patch1.85 KBcivicpixel

Comments

arthurf’s picture

The fix to the ui module is clearly need, so I've committed that. The remove of the step deletion however I'm a bit concerned about. The problem with removing that code is that the steps need to be pulled out of the step map table to prevent cruft build up- perhaps something like:

  /**
   * Completely deletes a configuration
   * @param $files
   *   boolean, should this configurations files be deleted?
   * @return boolean FALSE if delete conditions fail.
   */
  function delete($files = FALSE) {
    // Prepare to remove all steps
    foreach ($this->steps as $step) {
      if (! $step->remove_prepare()) {
        return FALSE;
      }
    }
    // Now remove the steps
    foreach ($this->steps as $step) {
      $this->step_remove($step->step_order);
    }
...

The problem still is that as these steps are removed from the step map, they also need to be checked for uniqueness- if no other configuration is using them they should probably be deleted... not quite sure on that one.

civicpixel’s picture

Status: Needs review » Fixed

Tested on our development and live servers for a week, marking as fixed.

Status: Fixed » Closed (fixed)

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