Fix clobbering other nav menu items

MegaphoneJon commented 4 years ago

Hey Eli,

This fixes a bug I ran into today.

You're adding your "Honeypot Settings" nav item dynamically rather than adding it as a record to civicrm_navigation - which is good.

However, you determine your Nav ID by checking the max ID in civicrm_navigation and using the next number. If another extension has already added a dynamic menu item, that Nav ID is taken, and your entry overwrites theirs (if they're both in the Administer menu).

I made two commits to keep things separate:

The first regenerates honeypot.civix.php, by re-running civix generate:module.
The second uses a helper function that exists in newer versions of honeypot.civix.php to avoid this exact issue.

Once I merged my new honeypot.civix.php I saw that you'd recently merged a newer version from Seamus, which also includes the helper function - but since I'd already done it, I included it.

MegaphoneJon commented 4 years ago

Re-reading this, I want to clarify - the original civix.php file was pre-2018. The one you merged 2 weeks ago is from 2018. The one in this PR is the most current version.

elisseck commented 4 years ago

@MegaphoneJon works for me and looks great - thanks for updating that! The timeline checks out to be fair it took me about 2 years to get through that PR and merge it big yikes xD

elisseck / com.elisseck.civihoneypot

Fix clobbering other nav menu items #13