Page MenuHomePhabricator
New Question
Ponder Q377

Is there a right way to define edit engines with separate "create" and "edit" modes?
OpenPublic
Actions

Asked by yelirekim on Apr 23 2016, 3:26 AM.

Details

Preempting Disclaimer: Yes we know we're off in experimental territory by merrily using edit engines for our own devices. We don't have a whole lot of them yet, but they are fun and useful to us since it's super common that people want both API and web interface support for the types of things we build here.

A scenario that has cropped up a few times is that when creating some type of object, there are attributes that can be set during creation, but should not be modified when editing. We found a way to support this that seems to be mostly inline with the intended function of the extension points / subclass structure, but was also awkward and obtuse enough that I could easily see it being clobbered in the near future. I also don't see any examples of this in Phacility code anywhere, which further makes me believe we're on shaky ground / wouldn't be able to "follow the leader" from upstream refactors of similar code.

A verbatim example is below. We've defined a release product, similar in purpose to the concept of a product in releeph. The product must have a repository associated with it when it's created, but you shouldn't be permitted to change the repository of a product after the fact. The edit engine definition below seems to behave as we want from the web and from the API.

<?php

final class CIReleaseProductEditEngine
  extends CIReleaseEditEngine {

  const ENGINECONST = 'gauntlet.product';
  const EDITENGINECONFIG_EDIT = 'edit';

  public function getEngineName() {
    return pht('Release Products');
  }

  protected function getObjectName() {
    return pht('Product');
  }

  protected function newEditableObject() {
    return CIReleaseProduct::initializeNewProduct();
  }

  protected function newObjectQuery() {
    return new CIReleaseProductQuery();
  }

  protected function getObjectCreateTitleText($object) {
    return pht('Create New Product');
  }

  protected function getObjectEditTitleText($object) {
    return pht('Edit %s', $object->getName());
  }

  protected function getObjectEditShortText($object) {
    return $object->getName();
  }

  protected function getObjectCreateShortText() {
    return pht('Create Product');
  }

  protected function newBuiltinEngineConfigurations() {
    $create = $this->newConfiguration()
      ->setBuiltinKey(self::EDITENGINECONFIG_DEFAULT)
      ->setIsEdit(false)
      ->setIsDefault(true)
      ->setName($this->getObjectCreateShortText());
    $edit = $this->newConfiguration()
      ->setBuiltinKey(self::EDITENGINECONFIG_EDIT)
      ->setIsEdit(true)
      ->setIsDefault(false)
      ->setFieldLocks([
          'repository' => PhabricatorEditEngineConfiguration::LOCK_LOCKED,
        ]);
    return array(
      $create,
      $edit,
    );
  }

  protected function buildCustomEditFields($object) {
    $repository_field = (new CIRepositoryEditField())
      ->setKey('repository')
      ->setLabel(pht('Repository'))
      ->setDescription(pht('Repository this product is produced from.'))
      ->setConduitTypeDescription(pht('PHID of a repository.'))
      ->setTransactionType(CIReleaseProductTransaction::TYPE_REPOSITORY)
      ->setIsRequired(true)
      ->setSingleValue($object->getRepositoryPHID());

    return array(
      $repository_field,
      id(new PhabricatorTextEditField())
        ->setKey('name')
        ->setLabel(pht('Name'))
        ->setDescription(pht('Product name.'))
        ->setConduitDescription(pht('Rename the product.'))
        ->setConduitTypeDescription(pht('New product name.'))
        ->setTransactionType(CIReleaseProductTransaction::TYPE_NAME)
        ->setValue($object->getName()),
      id(new PhabricatorTextEditField())
        ->setKey('slug')
        ->setLabel(pht('Slug'))
        ->setDescription(pht('Product slug.'))
        ->setConduitDescription(pht('Change the product slug.'))
        ->setConduitTypeDescription(pht('New product slug.'))
        ->setTransactionType(CIReleaseProductTransaction::TYPE_SLUG)
        ->setValue($object->getSlug()),
      id(new PhabricatorRemarkupEditField())
        ->setKey('description')
        ->setLabel(pht('Description'))
        ->setDescription(pht('Product description.'))
        ->setConduitDescription(pht('Change product description.'))
        ->setConduitTypeDescription(pht('New product description.'))
        ->setTransactionType(CIReleaseProductTransaction::TYPE_DESCRIPTION)
        ->setValue($object->getDescription()),
      id(new PhabricatorSelectEditField())
        ->setKey('status')
        ->setLabel(pht('Status'))
        ->setDescription(pht('Product status.'))
        ->setConduitDescription(pht('Change product statuc.'))
        ->setConduitTypeDescription(pht('New product status constant.'))
        ->setTransactionType(CIReleaseProductTransaction::TYPE_STATUS)
        ->setValue($object->getStatus())
        ->setOptions(CIReleaseProduct::getStatusNameMap()),
      id(new PhabricatorSelectEditField())
        ->setKey('strategy')
        ->setLabel(pht('Naming Strategy'))
        ->setDescription(pht('Product naming strategy.'))
        ->setConduitDescription(pht('Change product naming convention.'))
        ->setConduitTypeDescription(pht('New product naming strategy constant.'))
        ->setTransactionType(CIReleaseProductTransaction::TYPE_STRATEGY)
        ->setValue($object->getNamingStrategyKey())
        ->setOptions(CIReleaseNamingStrategy::getNamingStrategyMap()),
    );
  }

}

My question is, would you suggest a different way of doing this?

Show 1 Comment

Answers

epriestley
Updated 2,941 Days Ago
Actions

There's no One Right Way right now. This is a valid use case and has some support today but few examples. It should have more support and examples in the future.

Five possible non-exclusive approaches, briefly:

  1. Make buildCustomEditFields() do different things when creating/editing (example: ProjectEditEngine).
  2. Enforce the rule when applying transactions (example: TYPE_VCS in RepositoryEditor).
  3. Pass state to the EditEngine from the outside (example: AlmanacServiceEditEngine, but interacts poorly with the API).
  4. Provide multiple form templates (no examples today, as above).
  5. Write multiple EditEngines (no examples today, probably a terrible idea, included for completeness).

Some of these have major downsides/limits. For example, (2) will implement the rule correctly but users will have no clue it's a rule until they run afoul of it. (3) will get the UI correct but doesn't extend to the API.

I'd expect (1) to be the most useful here. Try having buildCustomEditFields() just do $field->setIsLocked(!$this->getIsCreate()) as a starting point and tossing the custom configurations -- I think that'll work.

We'll be doing this kind of thing more in the future and should have more formal structure and more examples to reference.

I expect us to use multiple default configurations more for really providing multiple forms (e.g., maybe "Create Public Event" vs "Create Private Event" in Calendar) than for specializing form edit phases, though.

Add a Comment

New Answer

Answer

Phabricator · Built by Phacility