Vanilla 1 is no longer supported or maintained. If you need a copy, you can get it here.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.
Options

[Bug] Duplication of discussion during editing

edited September 2006 in Vanilla 1.0 Help
I think I have found a bug when editing a discussion.

Everytime I edit the first comment of a discussion, and force a validation error (leaving the discussion topic, or the comments box empty) I get a warning and the same discussion form. But when I eliminate the validation error condition and save the changes, I get a new discussion with only one comment, instead of the original discussion being edited.

Moreover, if I force a second validation error, when editing a discussion I get the warning but the discussion form is transformed in a comment form, and if saved, it goes to the end of the original discussion as a normal comment.

I guess that some PostBack information is lost when a validation error happends in a discussion form.

Comments

  • Options
    MarkMark Vanilla Staff
    That's a bad bug if I can duplicate. I'll try it out when I get back.
  • Options
    Happens for me as well.
  • Options
    I discovered it when working on an extension which does some comment-body validations in the PostLoadData delegation of the DiscussionForm. I thought it was my problem, but I have checked it in fresh instalations of Vanilla 1.0 and 1.0.1
  • Options
    edited August 2006
    More information: The original link ends with a parameter: http://....../post.php?CommentID=....
    After a validation warning the parameter dissapears and the link looks like: http://....../post.php

    Edit: (Forget about this comment, I was not yet understanding how the postback information was added to the form)
  • Options
    edited August 2006
    I have found the problem. In Vanilla.Control.DiscussionForm.php, look at the lines:
    if ($this->PostBackAction == 'SaveDiscussion') { $this->Discussion->Clear(); $this->Discussion->GetPropertiesFromForm($this->Context);
    The Clear() operation erases the FirstCommentID and AuthUserID information in the Discussion object, which is afterwards used to generate the PostBack information in the GetDiscussionForm method.

    I have commented the line and it works fine. Is there any reason I'm missing to enforce the Clear() operation?

    Other solution could be to change the GetDiscussionForm method to get the PostBack information from the Comment object instead:
    // $this->PostBackParams->Set('CommentID', $Discussion->FirstCommentID); // $this->PostBackParams->Set('AuthUserID', $Discussion->AuthUserID); $this->PostBackParams->Set('CommentID', $Discussion->Comment->CommentID); $this->PostBackParams->Set('AuthUserID', $Discussion->Comment->AuthUserID);
    BTW, in the GetCommentForm setting the CommentID postback parameter is skipped. I have tried it also here, commenting the line which sets the CommentID directly, and it works. I don't know where it is done.

    What is the better solution? Skip the clearing or changing the setting of the PostBack parameters? I would say that the first, as more information about the discussion may be lost in the clearing which may be needed in a delegation.
  • Options
    MarkMark Vanilla Staff
    I actually despise that control. It is a rats nest, and I'd love to reprogram the constructor entirely. Which I may end up doing... We'll see. But thanks for looking into this.
  • Options
    MarkMark Vanilla Staff
    I've fixed this bug in development (and on this forum and vanilladev). Please feel free to test it out on vanilladev to make sure I got it right :)
  • Options
    @Mark:
    I have tried on this forum, having a look at the html generated when the saving attempts fail, and it is working perfectly for both, editing discussions and comments. The Discussion and Comment IDs are not lost anymore.

    Where may I have a look at the new code? I'm interested on what you did with that constructor.
    And, of course, thanks!
This discussion has been closed.