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

Issue #173: Converts contact categories to CMI #248

Closed
wants to merge 2 commits into from
Closed

Issue #173: Converts contact categories to CMI #248

wants to merge 2 commits into from

Conversation

behindthepage
Copy link
Contributor

drupal_write_record('contact', $form_state['values']);
if (isset($cid)) {
$c = array_search($cid, $cids);
$categories[$c] = $contact; // *************************************
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's with the //*** here? I think we can remove this.

@quicksketch
Copy link
Member

Hi @behindthepage, thanks again for taking this on. The changes are looking great! I left some inline feedback on the commit for minor things.

While testing this out locally, I found an issue where editing categories can unexpectedly overwrite other category settings. What I did:

  • Install contact module.
  • Add an additional category with the weight -10.
  • Edit the new category and re-save it with no changes. The new category will now show up twice, and the original category included with the module is deleted.

I think it has to do with the leniency in contact.categories.json between using the "cid" and the order of the array keys. It looks the order of items in contact.categories.json is also assumed to be the CID, but the CID appears to be explicitly defined. If you reorder the categories in the JSON file, the index of each category is used as the CID instead of the "cid" key explicitly defined. I'd suggest depending only on the explicit CID key and ignore the order of the array when dealing with loading/saving items.

@behindthepage
Copy link
Contributor Author

Thanks @quicksketch I will make those changes. I thought I had fixed the last problem but obviously not quite.

To my delight I found that the D7 Coder module works with minimal changes to info file. It was the origin of the strange white space but will change as you suggest. Have added a note above relating to hook_install.

I won't be present at future weekly video meetings as it is too early for me. zzzzzz. Pity about the time difference.

Regards
Geoff

@quicksketch
Copy link
Member

To my delight I found that the D7 Coder module works with minimal changes to info file.

Yay, for portability. :) I've ported a few modules as well as needed, right now it only takes few minutes to get the basics working in just about any module. Let's hope that continues.

I wrapped up backdrop/backdrop-issues#237 and all update numbers are now reverted back to 1xxx. When you next work on this could you pull from 1.x and change the update to be contact_update_1001?

@behindthepage
Copy link
Contributor Author

Need to do annother commit to delete conflict text.

quicksketch added a commit to quicksketch/backdrop that referenced this pull request Jan 7, 2015
quicksketch added a commit that referenced this pull request Jan 7, 2015
Issue #248: Removing all Drupal 8 references in TODOs.
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Sep 12, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Sep 14, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Sep 14, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Sep 14, 2016
quicksketch pushed a commit to quicksketch/backdrop that referenced this pull request Dec 15, 2023
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

Successfully merging this pull request may close these issues.

2 participants