studiopress / agentpress-listings

AgentPress Listings
http://wordpress.org/plugins/agentpress-listings/
GNU General Public License v2.0
9 stars 11 forks source link

Editing a Listings taxonomy results in a duplicate taxonomy with no ID #19

Closed nickcernis closed 9 years ago

nickcernis commented 9 years ago

To reproduce:

  1. Go to Listings > Register Taxonomies
  2. Add a new taxonomy with ID ‘bathrooms’, plural name ‘Bathrooms’, and singular name ‘Bathroom’
  3. Edit the taxonomy and change the plural name to ‘Bathroomz’

You’ll see that the edited taxonomy has a blank ID, and that the old unedited version is still present:

screen shot on 2015-01-01 at 14-47-09

The following two error messages are generated when the taxonomy is edited:

[28-Dec-2014 23:29:52 UTC] PHP Notice: Undefined variable: id in /vagrant/wp/wp-content/plugins/agentpress-listings/includes/class-taxonomies.php on line 200

[28-Dec-2014 23:29:52 UTC] PHP Notice: Undefined variable: id in /vagrant/wp/wp-content/plugins/agentpress-listings/includes/class-taxonomies.php on line 204

Attempting to delete the taxonomy results in a ‘nice try, partner’ message.

Modifying the edit_taxonomy() method in class_taxonomies.php to define $id before using it on line 197 appears to fix the issue (fix included in PR#15):

        $id = $args['id'];

        $args = array(
            'labels'       => $labels,
            'hierarchical' => true,
            'rewrite'      => array( 'slug' => $id ),
            'editable'     => 1
        );

        $tax = array( $id => $args );

Additionally, maybe it would be worth changing the message that people see upon attempting to delete a taxonomy with no ID, so that it sounds less accusatory/confusing? (We had a complaint about this.)

Perhaps instead of, “Nice try, partner. But that taxonomy doesn't exist. Click back and try again.”, simply: “Sorry, but that taxonomy could not be deleted.”

dreamwhisper commented 9 years ago

I confirmed this earlier this morning and verified the fix. @nathanrice

rrennick commented 9 years ago

Perhaps instead of, “Nice try, partner. But that taxonomy doesn't exist. Click back and try again.”, simply: “Sorry, but that taxonomy could not be deleted.”

I'm fairly sure that's a core WP message.

dreamwhisper commented 9 years ago

I'm fairly sure that's a core WP message.

It is. I missed that Nick suggested a change there.

dreamwhisper commented 9 years ago

No! I'm wrong - https://github.com/copyblogger/agentpress-listings/blob/c01ba501765adaa9cdc17dcaf5be9be0928d47fb/includes/views/edit-tax.php#L7 https://github.com/copyblogger/agentpress-listings/blob/c01ba501765adaa9cdc17dcaf5be9be0928d47fb/includes/class-taxonomies.php#L150

Modeled after an old core message (I think)

nickcernis commented 9 years ago

@rrennick @dreamwhisper Yeah, I thought it sounded very WordPress-ish at first, (Howdy, partner!), but then found it's something we could change in class-taxonomies.php.

Not a big deal, and my PR doesn't include a change for that, but thought I'd mention it nevertheless.

NicktheGeek commented 9 years ago

this can be tested.

To test

dreamwhisper commented 9 years ago

Confirmed x3 with support staff.