Fix issue processwire/processwire-issues#1082 as well as improve error handling when one places a fieldset closer before its opener

wire/modules/Fieldtype/FieldtypeFieldsetOpen.module

@@ -113,6 +113,60 @@ class FieldtypeFieldsetOpen extends Fieldtype {
 		return $field->type instanceof FieldtypeFieldsetOpen && !($field->type instanceof FieldtypeFieldsetClose);
 	}
 
+	/**
+	 * Check that Fieldgroup has matching open/close fieldsets and in correct order
+	 * 
+	 * @param Fieldgroup $fieldgroup
+	 * @return bool
+	 * @since 3.0.164
+	 * 
+	 */
+	public function checkFieldgroupFieldsets(Fieldgroup $fieldgroup) {
+		
+		list($openers, $closers, $isChanged) = array(array(), array(), false);
+
+		foreach($fieldgroup as $field) {
+			/** @var Fieldtype $fieldtype */
+			if(!$field->type instanceof FieldtypeFieldsetOpen) continue;
+			if($field->type instanceof FieldtypeFieldsetClose) {
+				if(!strpos($field->name, self::fieldsetCloseIdentifier)) continue;
+				$name = substr($field->name, 0, -1 * strlen(self::fieldsetCloseIdentifier));
+				$closers[$name] = $field;
+			} else if(!isset($closers[$field->name])) {
+				// ensure opener comes before closer, otherwise only closer remains
+				$openers[$field->name] = $field;
+			}
+		}
+
+		if(count($openers) === count($closers)) return true;
+
+		foreach($openers as $name => $opener) {
+			/** @var Field $opener */
+			if(isset($closers[$name])) continue;
+			$closer = $this->getFieldsetCloseField($opener);
+			if($closer) {
+				$fieldgroup->insertAfter($closer, $opener);
+			} else {
+				$fieldgroup->remove($opener);
+			}
+			$isChanged = true;
+		}
+		
+		foreach($closers as $name => $closer) {
+			/** @var Field $closer */
+			if(isset($openers[$name])) continue;
+			$opener = $this->getFieldsetOpenField($closer); 
+			if($opener) {
+				$fieldgroup->insertBefore($opener, $closer); 
+			} else {
+				$fieldgroup->remove($closer);
+			}
+			$isChanged = true;
+		}
+		
+		return $isChanged;
+	}
+
 	/**
 	 * Get the Field that closes/terminates the given Fieldset field
 	 * 
@@ -123,7 +177,7 @@ class FieldtypeFieldsetOpen extends Fieldtype {
 	 */
 	public function getFieldsetCloseField(Field $field, $createIfNotExists = false) {
 		
-		if(!$this->isFieldset($field)) return;
+		if(!$this->isFieldset($field)) return null;
 	
 		$name = $field->name . self::fieldsetCloseIdentifier;
 		$closer = $this->wire('fields')->get($name); 
@@ -154,6 +208,35 @@ class FieldtypeFieldsetOpen extends Fieldtype {
 		
 		return $closer; 
 	}
+	
+	/**
+	 * Get the Field that opens the given FieldsetClose field
+	 *
+	 * @param Field $field
+	 * @return null|Field of type FieldtypeFieldsetOpen, if found
+	 * @since 3.0.164
+	 *
+	 */
+	public function getFieldsetOpenField(Field $field) {
+
+		if(!$field->type instanceof FieldtypeFieldsetClose) return null;
+		if(!strpos($field->name, self::fieldsetCloseIdentifier)) return null;
+
+		$name = substr($field->name, 0, -1 * strlen(self::fieldsetCloseIdentifier));
+		$opener = $this->wire()->fields->get($name);
+
+		if($opener) return $opener;
+
+		foreach($this->wire()->fields as $f) {
+			if(!$f->type instanceof FieldtypeFieldsetOpen) continue;
+			$closeFieldID = (int) $f->get('closeFieldID');
+			if($closeFieldID != $field->id) continue;
+			$opener = $f;
+			break;
+		}
+
+		return $opener;
+	}
 
 	/**
 	 * Hook executed when field is added via ProcessField

wire/modules/Process/ProcessTemplate/ProcessTemplate.module

@@ -2755,9 +2755,14 @@ class ProcessTemplate extends Process {
 					$badFieldsets[$field->name . '_END'] = $field;
 				}
 			}
-			
+		
-			foreach($badFieldsets as $field) {
+			if(count($badFieldsets)) {
-				$this->error(sprintf($this->_('Error with placement of fieldset/tab "%s" - please fix and save again'), $field->name));
+				foreach($badFieldsets as $field) {
+					$this->error(sprintf($this->_('Error with placement of fieldset/tab "%s" - please fix and save again'), $field->name));
+				}
+				/** @var FieldtypeFieldsetOpen $fieldset */
+				$fieldset = $this->wire()->fieldtypes->get('FieldtypeFieldsetOpen');
+				if($fieldset->checkFieldgroupFieldsets($this->template->fieldgroup)) $saveFieldgroup = true;
 			}
 		}
 
@@ -2921,6 +2926,7 @@ class ProcessTemplate extends Process {
 		$removeIds = $this->wire('input')->get('fields'); 
 		if(empty($removeIds)) $this->session->redirect('./');
 		$removeIds = explode(',', $removeIds); 
+		$removeFields = array();
 
 		$fieldgroup = $this->template->fieldgroup; 
 		$this->wire('processHeadline', sprintf($this->_('Remove Fields from Template: %s'), $this->template->name)); 
@@ -2939,9 +2945,20 @@ class ProcessTemplate extends Process {
 		$checkboxes->icon = 'times-circle';
 		$checkboxes->attr('name', 'remove_fields'); 
 		$checkboxes->description = $this->_("You have asked to remove one or more fields from the template. This will result in data associated with the fields below being permanently deleted. If the fields that are removed contain a lot of data, it may take time for this operation to complete after you confirm and submit this form. Please confirm that you understand this and want to delete the field(s) by checking the boxes below.");
-
+		
 		foreach($fieldgroup as $field) {
-			if(!in_array($field->id, $removeIds)) continue; 
+			if(!in_array($field->id, $removeIds)) continue;
+			$removeFields[$field->id] = $field;
+			if($field->type instanceof FieldtypeFieldsetClose) {
+				$opener = $field->type->getFieldsetOpenField($field);
+				if($opener && $fieldgroup->hasField($opener) && !in_array($opener->id, $removeIds)) {
+					unset($removeFields[$field->id]); 
+					$removeFields[$opener->id] = $opener;
+				}
+			}
+		}
+
+		foreach($removeFields as $field) {
 			$checkboxes->addOption($field->id, sprintf($this->_('Remove field "%1$s" from template "%2$s"'), $field->name, $this->template->name)); 
 		}