We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 17284
    • 54 Posts
    modx rev: 2.3.1 with friendly urls.
    quip: 2.3.2-pl with recaptcha


    settings:
    [[!Quip? 
    &thread=`thread-[[*id]]` 
    &threaded=`0` 
    &useGravatar=`0`
    &tplComment=`tplQuipComment`
    &useCss=`0`]]
    [[!QuipReply? 
    &thread=`thread-[[*id]]`
    &useGravatar=`0`
    &tplAddComment=`tplQuipAddComment`
    &recaptcha=`1`
    &disableRecaptchaWhenLoggedIn=`0`
    &autoConvertLinks=`0`
    &moderate=`1`
    ]]
    


    This problem is so obvious that it must have come up before, but I can't find anything on it in google. I just added comments to our site pages, and after we poked around a bit debugging etc, this problem showed up:


    When a user posts a comment, with all fields valid, this is appended to the query string:

    ?quip_approved=0#qcom16

    I can't find where this redirect happens, it's not in the actual form html itself, it appears to be a redirect to that url during the form submission process.

    However, when you approve the comment, the query string remains, and quip keeps showing the message that the comment is waiting moderation, because the query string overrides any other internal checks quip does. It took me a long time to realize that a query string was being used for this purpose, I noticed it only by accident since it's such a fundamentally flawed idea I would never have thought to do it that way. The only way to make the message about moderation queue go away is to either manually remove the query string from the url then refresh the page, or to leave the page, then return, by clicking links, not using the back/forward actions of browser.

    This data should clearly not be fed to quip via query string, but through an internal check quip should be doing on loading the page, however if you remove it manually, then the page doesn't know about its moderation queue state. To my mind, this shoudl not be handled this way, but rather via session variable or something like that, which quip checks every page load, and which is based on the actual real moderation state, approved/not approved, or something like that.

    I don't see any real way to check this or fix it, but it seems like an issue so obvious that it must have come up before here, particularly obvious when you are testing/debugging since you will be refreshing the page.

    Note also, that if a user bookmarks a page before the comment is approved, then the message that the comment is being moderated will appear every time they access the bookmark, and similarly, no matter how many refreshes or subsequent new comments one makes, this message remains forever, or if other users post a new comment, that url will always force the incorrect rendering of the moderation queue message.

    My fear is that this points to a deeper set of logic errors with quip, since this problem is so obvious, and query strings should never have been used at all for this type of data.

    There's a preview query string attached by default as well.

    I am using templates for the comments and reply blocks, taken from the modx docs, simply removed some form fields and edited output html a bit.

    After testing a bit, I found a hack action that does 'fix' it, that's submitting an empty comment, which triggers errors, and which then gets rid of that query string data and replaces it with the default that the quip reply form has in it.

    [ed. note: lizardx last edited this post 9 years, 8 months ago.]
      • 17284
      • 54 Posts
      components/quip/controllers/web/ThreadReply.php:339
      components/quip/elements/snippets/snippet.quipreply.php:146
      
      if (isset($_GET['quip_approved']) && $_GET['quip_approved'] == 0) {
                  $this->setPlaceholder('successMsg',$this->modx->lexicon('quip.comment_will_be_moderated'));
      }
      

      This is the offending check, it's incomplete, and needs a further check to the actual moderation state of the user's comment (ie, approved/unapproved). I don't know what code does that, haven't found it yet.

      ie:

      if (isset($_GET['quip_approved']) && $_GET['quip_approved'] == 0) {
              if ( commentApproved ) {
                    $this->setPlaceholder('successMsg',$this->modx->lexicon('quip.comment_will_be_moderated'));
             else {
                 $this->setPlaceholder('successMsg',$this->modx->lexicon('quip.comment_approved'));
             }
      }
      


      or something like that, depending on how the comment approved state actually is determined internally.

      To fix that part is basically just a hack, since the real fix is to require cookies, dump all use of GET in quip, and replace those with sessions which are set each time things are changed.

      However, I'll see if I can generate a patch for this that will at least fix the error by checking for the internal state of moderation for that thread id. [ed. note: lizardx last edited this post 9 years, 8 months ago.]