Rename contact category to contact form

The rename makes a lot of sense to me and while it's a huge API change for anyone already using it, I doubt that many are doing that...

I can see how "Category" might be confusing/ambiguous, but "Page" is very ambiguous, too... (?)

If you were to place a contact form into a block, then the selector would ask you which "Page" you want to use.

I'd recommend to just simply name this a "Contact Form", because that's what it is...

That name would only get a bit confusing at the Entity API level under the hood - a "contact form" has multiple "form modes" etc.pp. - but not sure whether that low-level detail really matters.

@sun good point "form" makes the most sense. Going to start work on this patch today.

Contact form works as well for me.

Contact form it is.

@crowdcg are you still interested in working on this?

@larowlan, yes initial patch tomorrow. Have been working on it piecemeal, but think I need to just get a first patch up to iterate and get feedback on.

@andypost, et al. This is a simple patch, mainly focusing on the UI. I guess my big question is if all the "category" used in code should be changed to use "form" or "contactform" or something else. Also should the file names, classes, functions, etc. all be changed as well. I tried this holistic approach earlier in developing this patch but was unsuccessful. Feedback and direction greatly appreciated, at least this patch makes the interface less confusing.

Ouch, that's what I get for submitting that so late at night. Patch that doesn't break setup attached.

Yes, the entity type should be renamed to contact_form, form for the config prefix.

Not sure about the class name, just naming it "Form" seems like a bad idea, so possibly "ContactForm" ?

@berdir, thanks for the direction. Agreed on the second point as well. Will work on another patch incorporating those suggestions.

1. +++ b/core/modules/contact/config/schema/contact.schema.yml
    contact.category.*:
   
   @@ -29,7 +29,7 @@ contact.settings:
        default_category:
   
   +++ b/core/modules/contact/contact.local_actions.yml
    contact.category_add:
   
   +++ b/core/modules/contact/contact.menu_links.yml
    contact.category_list:
   
   +++ b/core/modules/contact/contact.routing.yml
        _entity_list: 'contact_category'
        _entity_form: 'contact_category.add'
        _entity_form: 'contact_category.edit'
   ...
    contact.site_page_category:
      path: '/contact/{contact_category}'
   ...
      requirements:
        _entity_access: 'contact_category.view'
   
   +++ b/core/modules/contact/src/Entity/Message.php
      public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
        $fields['category'] = FieldDefinition::create('entity_reference')
   

   Hm. Don't we need to update all of these, too?

2. -      '#title' => t('Make this the default category.'),
   +      '#title' => t('Make this the default form.'),
   

   (and elsewhere)

   A straight s/category/form/g replacement results in a lack of context in the UI strings. They should be replaced with "contact form" instead (which is a unique term).

3.    public function buildHeader() {
   -    $header['category'] = t('Category');
   +    $header['category'] = t('Form');
   

   The administrative listing is the only case where "form" may not be accurate, or at least results in a suboptimal language/presentation. Changing it into "Contact form" won't improve it either...

   We may want to ignore this issue; just wanted to point it out.

4. +++ b/core/modules/contact/src/Controller/ContactController.php
   -   *   contact category form.
   +   *   contact form form.
   

   Duplicate "form".

5. +++ b/core/modules/contact/src/Tests/ContactSitewideTest.php
   @@ -69,13 +69,13 @@ function testSiteWideContact() {
   +    // Delete old fiorms to ensure that new forms are used.
   

   Typo in "fiorms"

Thanks @sun! Resolved all five I believe in this patch.

Need to pay more attention...trying again!

Hopefully this patch resolves most of the errors and exceptions.

One more time with feeling!

1. +++ b/core/modules/contact/config/schema/contact.schema.yml
   @@ -1,8 +1,8 @@
   -contact.category.*:
   +contact.form.*:
   
   +++ b/core/modules/contact/src/Entity/ContactForm.php
   @@ -0,0 +1,80 @@
   + *   id = "contact_form",
   ...
   + *   config_prefix = "contact_form",
   

   The schema doesn't match with the config_prefix.

   Not 100% sure, but I do think that a config prefix of "config_form" is better than just form (which means you can leave out the explicit definition, as the default is the entity type ID anyway). So update the schema and the default config files to contact.contact_form.

2. +++ b/core/modules/contact/src/ContactForm.php
   @@ -0,0 +1,125 @@
   +class ContactForm extends EntityForm {
   

   I guess this is really a ContactFormForm class now? A form for editing contact forms :) Or maybe ContactFormEditForm?

   Also, please set up your git configuration to better track renames, look for renames = copies in https://www.drupal.org/documentation/git/configure. Makes the patch much smaller and easier to review.

1. That said, form is probably not more generic than "type", so that might work too and is possibly easier to change. @larowlan, @andypost?

"type" isn't really clear on what it is. A [contact] "form" is a form. A [contact] type is… a "friend", "co-worker", etc category. — Let's avoid the "node type" trap (double-meaningless).

I think we should keep "contact form" in UI strings. However, if "form" presents an issue at the code level, then alternative synonyms are possible - whichever synonym we choose though, it should transport the same meaning. E.g.: channel, funnel, tunnel, connection, junction, etc.

I think you misunderstood me :)

I did not mean to change form to type, I'm very +1 on form, the UI strings, entity class and entity type ID should be contact form/contact_form/ContactForm.

I'm *only* wondering if the config prefix should be config_form or just form. form seems very generic, but I was just making the point that if node.type is OK (and custom_block.type, and comment.type), then contact.form is probably OK too.

@Berdir, thank you for the feedback and helpful link to git configuration. I've adjusted my .gitconfig and .gitignore, so patches should be smoother from here on out!

1. I've updated the contact.schema.yml, but just so I'm clear, the files contact.form.* under contact/config/install should also now be contact.contact_form.* correct?

2. Personally, I think ContactFormEditForm is clearer than ContactFormForm even if it is more verbose :)

1. Yeah, whether we chose to go with contact_form or just form, the schema, default config files and config_prefix/id in the entity type annotation all need to align. Then hopefully the test fails will go down :)

2. Yeah, the only problem with that is that form would also be used for adding contact forms :)

Sounds like we really need a different noun under the hood. "form" is definitely problematic. And before renaming it to "type", I'd rather keep the current "category"... :P

We also need to watch out: English is both a colorful but also poor language, so it's easy to fall into the trap of using words that don't exist, don't have a meaning, and worse, which are not translatable into other languages.

How about this? — contact point

In essence, we're talking about a thing that defines a communication recipient and available information that may be communicated to the recipient for a certain purpose. The fact that communication happens through a web form is not really relevant (and might not necessarily be true, who knows…?). We're defining a possible point of contact, or contact point.

I'm not fussy, but if I had to choose I'd keep the prefix as contact_form instead of form or type.
+1 to ContactFormEditForm instead of ContactForm for the form.

go listen to the discussion

Lets see whats still broken

Local tests came back

1. +++ b/core/modules/contact/contact.links.action.yml
   @@ -1,6 +1,6 @@
   -contact.category_add:
   -  route_name: contact.category_add
   -  title: 'Add category'
   +contact.form_add:
   +  route_name: contact.form_add
   +  title: 'Add form'
   

   Should we use "Add contact form" here and possibly in other UI strings?

2. +++ b/core/modules/contact/contact.links.menu.yml
   @@ -1,8 +1,8 @@
      parent: system.admin_structure
   -  description: 'Create a system contact form and set up categories for the form to use.'
   -  route_name: contact.category_list
   +  description: 'Create a system contact form and set up other forms to use.'
   +  route_name: contact.form_list
   

   I don't think the new description makes sense? It was about a single system contact form with categories.

   Can be simplified to just "Create and manage contact forms" or something like that.

3. +++ b/core/modules/contact/contact.routing.yml
   @@ -1,51 +1,51 @@
   -    _entity_form: 'contact_category.add'
   -    _title: 'Add category'
   +    _entity_form: 'contact_form.add'
   +    _title: 'Add form'
   

   Same here.

4. +++ b/core/modules/contact/contact.routing.yml
   @@ -1,51 +1,51 @@
      defaults:
   -    _title: 'Contact category form'
   +    _title: 'Contact form'
   

   Not sure this makes sense either :)

5. +++ b/core/modules/contact/src/ContactFormInterface.php
   @@ -0,0 +1,17 @@
   + * Provides an interface defining a contact form entity.
   

   Was like this before I guess, but seems strange, it *is* an interface, i doesn't provide one? Just "Interface for contact form entities" or so?

6. +++ b/core/modules/contact/src/Entity/ContactForm.php
   @@ -2,7 +2,7 @@
     * @file
   - * Definition of Drupal\contact\Entity\Category.
   + * Definition of Drupal\contact\Entity\ContactForm.
   

   While touching, change to Contains \Drupal.. ?

7. +++ b/core/modules/contact/src/Form/ContactFormDeleteForm.php
   @@ -42,8 +42,8 @@ public function getConfirmText() {
   -    drupal_set_message($this->t('Category %label has been deleted.', array('%label' => $this->entity->label())));
   -    $this->logger('contact')->notice('Category %label has been deleted.', array('%label' => $this->entity->label()));
   +    drupal_set_message($this->t('Form %label has been deleted.', array('%label' => $this->entity->label())));
   +    $this->logger('contact')->notice('Form %label has been deleted.', array('%label' => $this->entity->label()));
   

   "Contact form"?

8. +++ b/core/modules/system/src/Tests/Ajax/DialogTest.php
   @@ -57,7 +57,7 @@ public function testDialog() {
          'dialogOptions' => array(
            'modal' => TRUE,
   -        'title' => 'Add category',
   +        'title' => 'Add form',
          ),
   

   Looks like this is just a test, but still, "contact form" ? ;)

Looks nice to me, would be good to have an API change approval but this is RTBC IMHO, makes it much clearer what those things actually are.

Only part that might need discussion is the config prefx, I'm ok with either form or contact_form.

rtbc
screenshot from manual testing

Seems to work fine for me as well. A small nitpick...the term "Label" for the title of the form seems strange to me.

I think we need a change record to tell everyone contact categories and now contact forms.

Change records that need an update:

• https://www.drupal.org/node/2023711
• https://www.drupal.org/node/1818734

Committed 093ea79 and pushed to 8.0.x. Thanks!

There's an open issue (issue #2091395) related to the hook_help() text of this module that relates to the changes in this issue. Cross-linking them on the advice of sprint mentor.