Conpherence - make empty comment submission behave like other apps
Summary: now we get a "you can't submit no text" error. Also puts the participant status updating inside the editor. Test Plan: made empty comments and got the right error dialogue. made legit comments and they went through. made a new conpherence - work. edited title + picture on old conpherence - worked. tried to submit non-updates to title and image - correct error. Reviewers: epriestley, chad Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2419 Differential Revision: https://secure.phabricator.com/D4734
This commit is contained in:
		@@ -44,6 +44,7 @@ final class ConpherenceUpdateController extends
 | 
				
			|||||||
          'ip' => $request->getRemoteAddr()
 | 
					          'ip' => $request->getRemoteAddr()
 | 
				
			||||||
        ));
 | 
					        ));
 | 
				
			||||||
      $editor = id(new ConpherenceEditor())
 | 
					      $editor = id(new ConpherenceEditor())
 | 
				
			||||||
 | 
					        ->setContinueOnNoEffect($request->isContinueRequest())
 | 
				
			||||||
        ->setContentSource($content_source)
 | 
					        ->setContentSource($content_source)
 | 
				
			||||||
        ->setActor($user);
 | 
					        ->setActor($user);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -55,28 +56,6 @@ final class ConpherenceUpdateController extends
 | 
				
			|||||||
            $conpherence,
 | 
					            $conpherence,
 | 
				
			||||||
            $message
 | 
					            $message
 | 
				
			||||||
          );
 | 
					          );
 | 
				
			||||||
          $time = time();
 | 
					 | 
				
			||||||
          $conpherence->openTransaction();
 | 
					 | 
				
			||||||
          $xactions = $editor->applyTransactions($conpherence, $xactions);
 | 
					 | 
				
			||||||
          $last_xaction = end($xactions);
 | 
					 | 
				
			||||||
          $xaction_phid = $last_xaction->getPHID();
 | 
					 | 
				
			||||||
          $behind = ConpherenceParticipationStatus::BEHIND;
 | 
					 | 
				
			||||||
          $up_to_date = ConpherenceParticipationStatus::UP_TO_DATE;
 | 
					 | 
				
			||||||
          $participants = $conpherence->getParticipants();
 | 
					 | 
				
			||||||
          foreach ($participants as $phid => $participant) {
 | 
					 | 
				
			||||||
            if ($phid != $user->getPHID()) {
 | 
					 | 
				
			||||||
              if ($participant->getParticipationStatus() != $behind) {
 | 
					 | 
				
			||||||
                $participant->setBehindTransactionPHID($xaction_phid);
 | 
					 | 
				
			||||||
              }
 | 
					 | 
				
			||||||
              $participant->setParticipationStatus($behind);
 | 
					 | 
				
			||||||
              $participant->setDateTouched($time);
 | 
					 | 
				
			||||||
            } else {
 | 
					 | 
				
			||||||
              $participant->setParticipationStatus($up_to_date);
 | 
					 | 
				
			||||||
              $participant->setDateTouched($time);
 | 
					 | 
				
			||||||
            }
 | 
					 | 
				
			||||||
            $participant->save();
 | 
					 | 
				
			||||||
          }
 | 
					 | 
				
			||||||
          $updated = $conpherence->saveTransaction();
 | 
					 | 
				
			||||||
          break;
 | 
					          break;
 | 
				
			||||||
        case 'metadata':
 | 
					        case 'metadata':
 | 
				
			||||||
          $xactions = array();
 | 
					          $xactions = array();
 | 
				
			||||||
@@ -112,23 +91,25 @@ final class ConpherenceUpdateController extends
 | 
				
			|||||||
              ->setTransactionType(ConpherenceTransactionType::TYPE_TITLE)
 | 
					              ->setTransactionType(ConpherenceTransactionType::TYPE_TITLE)
 | 
				
			||||||
              ->setNewValue($title);
 | 
					              ->setNewValue($title);
 | 
				
			||||||
          }
 | 
					          }
 | 
				
			||||||
 | 
					 | 
				
			||||||
          if ($xactions) {
 | 
					 | 
				
			||||||
            $conpherence->openTransaction();
 | 
					 | 
				
			||||||
            $xactions = $editor
 | 
					 | 
				
			||||||
              ->setContinueOnNoEffect(true)
 | 
					 | 
				
			||||||
              ->applyTransactions($conpherence, $xactions);
 | 
					 | 
				
			||||||
            $updated = $conpherence->saveTransaction();
 | 
					 | 
				
			||||||
          } else if (empty($errors)) {
 | 
					 | 
				
			||||||
            $errors[] = pht(
 | 
					 | 
				
			||||||
              'That was a non-update. Try cancel.'
 | 
					 | 
				
			||||||
            );
 | 
					 | 
				
			||||||
          }
 | 
					 | 
				
			||||||
          break;
 | 
					          break;
 | 
				
			||||||
        default:
 | 
					        default:
 | 
				
			||||||
          throw new Exception('Unknown action: '.$action);
 | 
					          throw new Exception('Unknown action: '.$action);
 | 
				
			||||||
          break;
 | 
					          break;
 | 
				
			||||||
      }
 | 
					      }
 | 
				
			||||||
 | 
					      if ($xactions) {
 | 
				
			||||||
 | 
					        try {
 | 
				
			||||||
 | 
					          $xactions = $editor->applyTransactions($conpherence, $xactions);
 | 
				
			||||||
 | 
					          $updated = true;
 | 
				
			||||||
 | 
					        } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
 | 
				
			||||||
 | 
					          return id(new PhabricatorApplicationTransactionNoEffectResponse())
 | 
				
			||||||
 | 
					            ->setCancelURI($this->getApplicationURI($conpherence_id.'/'))
 | 
				
			||||||
 | 
					            ->setException($ex);
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
 | 
					      } else if (empty($errors)) {
 | 
				
			||||||
 | 
					        $errors[] = pht(
 | 
				
			||||||
 | 
					          'That was a non-update. Try cancel.'
 | 
				
			||||||
 | 
					        );
 | 
				
			||||||
 | 
					      }
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    if ($updated) {
 | 
					    if ($updated) {
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -128,6 +128,7 @@ final class ConpherenceViewController extends
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    $form =
 | 
					    $form =
 | 
				
			||||||
      id(new AphrontFormView())
 | 
					      id(new AphrontFormView())
 | 
				
			||||||
 | 
					      ->setWorkflow(true)
 | 
				
			||||||
      ->setAction($this->getApplicationURI('update/'.$conpherence->getID().'/'))
 | 
					      ->setAction($this->getApplicationURI('update/'.$conpherence->getID().'/'))
 | 
				
			||||||
      ->setFlexible(true)
 | 
					      ->setFlexible(true)
 | 
				
			||||||
      ->setUser($user)
 | 
					      ->setUser($user)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -118,6 +118,27 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
 | 
				
			|||||||
          );
 | 
					          );
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
        $editor->save();
 | 
					        $editor->save();
 | 
				
			||||||
 | 
					        // fallthrough
 | 
				
			||||||
 | 
					      case PhabricatorTransactions::TYPE_COMMENT:
 | 
				
			||||||
 | 
					        $xaction_phid = $xaction->getPHID();
 | 
				
			||||||
 | 
					        $behind = ConpherenceParticipationStatus::BEHIND;
 | 
				
			||||||
 | 
					        $up_to_date = ConpherenceParticipationStatus::UP_TO_DATE;
 | 
				
			||||||
 | 
					        $participants = $object->getParticipants();
 | 
				
			||||||
 | 
					        $user = $this->getActor();
 | 
				
			||||||
 | 
					        $time = time();
 | 
				
			||||||
 | 
					        foreach ($participants as $phid => $participant) {
 | 
				
			||||||
 | 
					          if ($phid != $user->getPHID()) {
 | 
				
			||||||
 | 
					            if ($participant->getParticipationStatus() != $behind) {
 | 
				
			||||||
 | 
					              $participant->setBehindTransactionPHID($xaction_phid);
 | 
				
			||||||
 | 
					            }
 | 
				
			||||||
 | 
					            $participant->setParticipationStatus($behind);
 | 
				
			||||||
 | 
					            $participant->setDateTouched($time);
 | 
				
			||||||
 | 
					          } else {
 | 
				
			||||||
 | 
					            $participant->setParticipationStatus($up_to_date);
 | 
				
			||||||
 | 
					            $participant->setDateTouched($time);
 | 
				
			||||||
 | 
					          }
 | 
				
			||||||
 | 
					          $participant->save();
 | 
				
			||||||
 | 
					        }
 | 
				
			||||||
        break;
 | 
					        break;
 | 
				
			||||||
      case ConpherenceTransactionType::TYPE_PARTICIPANTS:
 | 
					      case ConpherenceTransactionType::TYPE_PARTICIPANTS:
 | 
				
			||||||
        foreach ($xaction->getNewValue() as $participant) {
 | 
					        foreach ($xaction->getNewValue() as $participant) {
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user