boonebgorges / buddypress-docs

GNU General Public License v3.0
106 stars 44 forks source link

Doc edit triggers new doc creation with `-2` suffix #653

Closed MatthewWilliamBailey closed 5 years ago

MatthewWilliamBailey commented 5 years ago

Hi Boone, Let me begin by saying we are quite willing to pay you for your help with our issue. Also, I apologize if this issue has been addressed already, I have searched and been unable to find mention of it here or anywhere else.

Here we go: When a user edits and saves a document, a new permalink is created (ex: https://tecconnect.org/docs/dec-2018-media-monitoring-report-opioids-and-substance-use/ becomes https://tecconnect.org/docs/dec-2018-media-monitoring-report-opioids-and-substance-use-2/, then https://tecconnect.org/docs/dec-2018-media-monitoring-report-opioids-and-substance-use-3/).

Is there a way to stop this from happening? Or is this the intended outcome for doc editing? Also, in the process of editing documents, if there was an attachment to the previous version, it gets dropped in the new saved version. Is there a way to keep the attachment when editing docs?

Thank you for your assistance and for the very helpful plugin.

dcavins commented 5 years ago

Hi Matthew-

Can you tell us more about your server environment and software used? I've been trying to recreate this issue from a post on the WP support forums, and cannot duplicate the issue.

boonebgorges commented 5 years ago

Hi @MatthewWilliamBailey - This is not intended behavior, and it's something we've unsuccessfully tried to diagnose in the past, as @dcavins has noted. (The attachment issue is a consequence of the renaming issue.)

Please share as much info as you're able to about your WP setup, especially regarding your theme and any customizations you may have made to it. If we're unable to track down the issue, maybe we could arrange to give me or @dcavins server access so that we can debug in an environment that demonstrates the problem.

MatthewWilliamBailey commented 5 years ago

Boone & David, Wow! I am impressed to hear back so soon, thank you both.

Website: https://tecconnect.org/ (requires login to access. Would you like WP Admin access? I am happy to provide if needed)

Hosted on GoDaddy Managed WordPress (Ultimate)

Theme: Woffice https://woffice.io/ (I had a child theme set up and working great but had some problems with that after recent updates so I deactivated)

Plugins: bbp style pack bbPress bbPress - Sort topic replies BuddyPress BuddyPress Docs BuddyPress Group Email Subscription Classic Editor DP Pro Event Calendar Enable Media Replace Google Analytics Dashboard for WP (GADWP) Google Analytics for WordPress by MonsterInsights Inactive Logout rtMedia for WordPress, BuddyPress and bbPress Sign-up Sheets Pro Unyson User Role Editor WordPoints WP Mail SMTP WPT Custom Mo File

Everything is current as far as software updates, and everything was working quite well until recent updates. I also have a problem with Private Messages not sending since I updated my theme and Buddypress. If you can work that out too, we’d be happy to pay you for your time.

Thanks again,

Matthew Bailey Marketing Media Web Specialist ANTHC | EpiCenter & DCHS (907) 729-3927

From: Boone Gorges notifications@github.com Reply-To: boonebgorges/buddypress-docs reply@reply.github.com Date: Tuesday, February 12, 2019 at 6:55 PM To: boonebgorges/buddypress-docs buddypress-docs@noreply.github.com Cc: "Bailey, Matthew W" mwbailey@anthc.org, Mention mention@noreply.github.com Subject: [External] Re: [boonebgorges/buddypress-docs] Buddypress Doc Editing (#653)

Hi @MatthewWilliamBaileyhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_MatthewWilliamBailey&d=DwMFaQ&c=L0NelKDfGwIEbGetsCr_dMk7qtG-8g4veA-ghIDZOcE&r=lWpcdEIhJIRfgEyfnB3k3VuyzHCMdPJLpsT0iw3s8lw&m=cp6IQr5NhNc8VttAEcZMFvt1WznQ_wQV39KAWYPgzUU&s=vEn5bTsdX8Fiaj5njkpD57BQGO7Nqdw6Kv0qIWvJAnU&e= - This is not intended behavior, and it's something we've unsuccessfully tried to diagnose in the past, as @dcavinshttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_dcavins&d=DwMFaQ&c=L0NelKDfGwIEbGetsCr_dMk7qtG-8g4veA-ghIDZOcE&r=lWpcdEIhJIRfgEyfnB3k3VuyzHCMdPJLpsT0iw3s8lw&m=cp6IQr5NhNc8VttAEcZMFvt1WznQ_wQV39KAWYPgzUU&s=o3h7p6njS21fgXeBsPUKj6Mvk9n4uwbxO0jOJTW6AW0&e= has noted. (The attachment issue is a consequence of the renaming issue.)

Please share as much info as you're able to about your WP setup, especially regarding your theme and any customizations you may have made to it. If we're unable to track down the issue, maybe we could arrange to give me or @dcavinshttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_dcavins&d=DwMFaQ&c=L0NelKDfGwIEbGetsCr_dMk7qtG-8g4veA-ghIDZOcE&r=lWpcdEIhJIRfgEyfnB3k3VuyzHCMdPJLpsT0iw3s8lw&m=cp6IQr5NhNc8VttAEcZMFvt1WznQ_wQV39KAWYPgzUU&s=o3h7p6njS21fgXeBsPUKj6Mvk9n4uwbxO0jOJTW6AW0&e= server access so that we can debug in an environment that demonstrates the problem.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_boonebgorges_buddypress-2Ddocs_issues_653-23issuecomment-2D463047311&d=DwMFaQ&c=L0NelKDfGwIEbGetsCr_dMk7qtG-8g4veA-ghIDZOcE&r=lWpcdEIhJIRfgEyfnB3k3VuyzHCMdPJLpsT0iw3s8lw&m=cp6IQr5NhNc8VttAEcZMFvt1WznQ_wQV39KAWYPgzUU&s=y2M5sWSA-YxBq7GF1V9msBpjgNQ1Vad9tR28V4qoBz0&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_As-2DRdfeG8XkYo0Nf6NtArJCP4qajMr1zks5vM4ybgaJpZM4a4bMv&d=DwMFaQ&c=L0NelKDfGwIEbGetsCr_dMk7qtG-8g4veA-ghIDZOcE&r=lWpcdEIhJIRfgEyfnB3k3VuyzHCMdPJLpsT0iw3s8lw&m=cp6IQr5NhNc8VttAEcZMFvt1WznQ_wQV39KAWYPgzUU&s=X1xd75fLu2WgSFeJ7IZQf4sMkAFVWMHd1S9Xt5gdliw&e=.

boonebgorges commented 5 years ago

Thanks, @MatthewWilliamBailey . I'm unfamiliar with the Woffice theme, but looking at their site, I see that they have baked-in BuddyPress support. This makes me suspect that they have custom Docs template, which in turn makes me suspect that there's an error in the template that's triggering this issue.

As a first step, could you send me a zip file containing the Woffice theme? Send it to [my github username] at gmail.com.

MatthewWilliamBailey commented 5 years ago

I shared a Dropbox folder with you.

boonebgorges commented 5 years ago

Thanks. I've identified the issue, which I believe is a bug in Woffice. Something similar is probably happening in other cases reported on wordpress.org.

Technical details: When the Doc edit page loads, it loads the WordPress media modal, which is used for the attachments feature. As part of this process, it detects which item you're currently editing by looking at wp.media.model.settings.post.id. This value, in turn, is determined by the post values passed to wp_enqueue_media(); see https://github.com/boonebgorges/buddypress-docs/blob/788bee974d27cf34f99b95ff6cd40d3ff1872343/includes/templatetags.php#L2074.

In the case of Woffice, it appears that wp_enqueue_media() is called early in the loading process, before Docs has had a chance to call the function. It does so without specifying a post value. See woffice/inc/static.php, around line 104. Since WP allows these values only to be set once, Docs cannot override it. As a result, wp.media.model.settings.post.id is empty.

This empty value tricks Docs into thinking that you're creating a Doc rather than editing an existing one; it triggers an AJAX request that results in a "dummy" doc being created, with that new ID sent back to the browser. This is why new Docs get created when saving.

I say this is a bug in Woffice for a few reasons:

  1. It appears that wp_enqueue_media() is being called on every front-end page. This, whether the user is logged in or not, and whether the media modal is required or not.
  2. When Woffice calls wp_enqueue_media(), it doesn't attempt to pass in the values of the current post. This breaks any other plugin that attempts to use the media modal later in the pageload.

It could also be that Docs could implement some strategy that'd make us more resilient in this kind of case. Here's a drop-in function that fixes the issue:

add_filter(
    'media_view_settings',
    function( $settings, $post ) {
        if ( ! bp_docs_is_existing_doc() ) {
            return $settings;
        }

        if ( ! empty( $settings['post']['id'] ) ) {
            return $settings;
        }

        $current_doc = bp_docs_get_current_doc();

        $settings['post'] = array(
            'id'    => $current_doc->ID,
            'nonce' => wp_create_nonce( 'update-post_' . $current_doc->ID ),
        );

        return $settings;
    },
    10,
    2
);

Possibly we could do something like this in Docs itself, to protect against this problem (which would occur anytime a plugin or theme calls wp_enqueue_media() improperly before Docs does). I'm unsure whether this would have knock-on effects - @dcavins maybe you have some thoughts about that. In the meantime, @MatthewWilliamBailey and others experiencing this problem can use the code above to work around the issue, and you might consider passing these details along to the Woffice support team.

MatthewWilliamBailey commented 5 years ago

Cool! So I should add this drop-in function in the static.php file before line 104, correct?

MatthewWilliamBailey commented 5 years ago

Looks like it worked. Huzzah!

boonebgorges commented 5 years ago

@MatthewWilliamBailey Glad to hear that it worked! Note that it should probably not go in static.php, because it'll be overwritten the next time you get a Woffice update. Safest bet is to create a bp-custom.php file (if it doesn't yet exist) and put it there. https://codex.buddypress.org/themes/bp-custom-php/

After mulling it over a bit, I think there's probably no harm in doing something like this in Docs, so I'll try to implement for the next bugfix release.

dcavins commented 5 years ago

Boone, I don't foresee any problems with adding a media_view_settings filter as you've proposed above. If anything, it will be conservative because of the bp_docs_is_existing_doc() check (I can imagine that some theme/plugin will be calling wp_enqueue_media() so early that the existing doc check will return false when it should be true).

I'm wondering if this fix might also address this issue (bad upload nonces): https://wordpress.org/support/topic/http-error-when-uploading-files-attachment-upload-not-working/

Thanks!

MatthewWilliamBailey commented 5 years ago

You guys are rock stars!

boonebgorges commented 5 years ago

Thanks, all. I've pushed up a fix, which will be included in 2.1.3.