Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert Contact categories to CMI #173

Closed
jenlampton opened this issue Feb 7, 2014 · 13 comments
Closed

Convert Contact categories to CMI #173

jenlampton opened this issue Feb 7, 2014 · 13 comments

Comments

@jenlampton
Copy link
Member

Contact categories need to save settings into a config file.

For reference, here is the parallel issue from drupal.org

@jenlampton jenlampton mentioned this issue Feb 7, 2014
28 tasks
@behindthepage
Copy link

I will do this.

@behindthepage
Copy link

In image.module all image styles are cached.

Should I cache all the contact categories?

Regards
Geoff

@behindthepage
Copy link

How does one retrieve the (multiple) categories that simpletest has added with the following function?

  function addCategory($category, $recipients, $reply, $selected) {
    $edit = array();
    $edit['category'] = $category;
    $edit['recipients'] = $recipients;
    $edit['reply'] = $reply;
    $edit['selected'] = ($selected ? '1' : '0');
    $this->drupalPost('admin/structure/contact/add', $edit, t('Save'));
  }

I assume they are not stored with the sites files e.g. contact.categories.website_feedback.json. Then where are they?

Confused.
Geoff

@quicksketch
Copy link
Member

Thanks @behindthepage! Image styles are cached because they're needed on most page loads. Contact categories have limited usage by comparison. You should think about needing caching for config files in the same way you should think about a database query.

However, unlike database queries, we don't have the ability to load mulitple rows at the same time. If contact categories are stored in a single file, then caching probably isn't needed. If each contact category were to need separate configuration, caching the overall list of configurations would probably be a good idea.

@quicksketch
Copy link
Member

I assume they are not stored with the sites files e.g. contact.categories.website_feedback.json. Then where are they?

You should be able to use config() and config_get() within your simple tests to load the config used by the sandbox installations. Within a test, config() will load the config files from the sandbox rather than the parent Drupal installation. During a test run, each test is given an independent files directory in which its config files are stored. This matches the behavior of how variable_get/set worked within tests in D7, and the same for database queries. Anything you do within a test class will affect the test sandbox, rather than the parent install.

@behindthepage
Copy link

Thanks @quicksketch.
Each Contact category has it's own config file like this

{
  "_config_name": "contact.categories.test_2",
  "cid": "test_2",
  "category": "Test 2",
  "recipients": "[email protected]",
  "reply": "",
  "weight": "6"
}

Loading them all only occurs twice.

  1. When the site wide contact form is loaded.
  2. When the Categories admin page is loaded.

I thought they might be cached but I don't know enough about the caching vs loading config files.

What think ye?
Geoff

@behindthepage
Copy link

Hi Nate,

Don't worry about answering this last post. D8 has separate category files but it might be better to have them all in one. Let me play around with it a bit more and come back to you with my findings.

Regards
Geoff

@quicksketch
Copy link
Member

D8 has separate category files but it might be better to have them all in one. Let me play around with it a bit more and come back to you with my findings.

Excellent, I was thinking about possibly suggesting the same thing. If contact categories required more configuration, separate files might make sense. With the minimal configuration available, a single config file might make sense for starters, avoiding the need for caching.

@behindthepage
Copy link

Hi @quicksketch,

I see some changes have been made to version numbers and I am close to doing a pull request for this issue.

Do I make version number in info file core = 1.x and contact_update_1000? Or should it still 8.x and 8000?

Regards
Geoff

@quicksketch
Copy link
Member

Hi @behindthepage, the core version numbers have changed, so yes you'll need to use core = 1.x in the .info file. You should just be able to git merge 1.x to have those changes pull in.

On update numbers we're really close to resetting those back to 1000. There's an issue for that here: #237. However, until that's part of the main 1.x branch, you should keep using 8000 as the update number.

@behindthepage
Copy link

Do I need to add hook_config_info()? e.g.

/**
 * Implements hook_config_info().
 */
function contact_config_info() {
  $prefixes['contact.settings'] = array(
    'label' => t('Contact settings'),
    'group' => t('Contact'),
  );
  $prefixes['contact.categories'] = array(
    'label' => t('Contact categories'),
    'group' => t('Contact'),
  );
  return $prefixes;
}

Regards
Geoff

@quicksketch
Copy link
Member

Do I need to add hook_config_info()? e.g.

Yep! Looks pretty good as is in your example. If contact categories were stored in separate files, you'd use the name_key and label_key in the hook instead. But since contact categories are all in one file like this, that's exactly how it should be. :)

@quicksketch
Copy link
Member

This has been fixed in backdrop/backdrop#271. Woot! Thanks @behindthepage!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants