Skip to content

Make comment private when changing status from moderation or cancelled#487

Merged
KaiNissen merged 1 commit into
masterfrom
privcomment
Jun 13, 2016
Merged

Make comment private when changing status from moderation or cancelled#487
KaiNissen merged 1 commit into
masterfrom
privcomment

Conversation

@JeroenDeDauw

Copy link
Copy Markdown
Contributor

Finishes https://phabricator.wikimedia.org/T137328

Please verify the logic is correct. What I did is check if the status is either
moderation or cancelled, as opposed to checking if the status is not external
incomplete as described in the ticket. I suspect these are equivalent, though
am not sure as the later is not very explicit.

Finishes https://phabricator.wikimedia.org/T137328

Please verify the logic is correct. What I did is check if the status is either
moderation or cancelled, as opposed to checking if the status is not external
incomplete as described in the ticket. I suspect these are equivalent, though
am not sure as the later is not very explicit.
private function makeCommentPrivate() {
$this->comment = new DonationComment(
$this->comment->getCommentText(),
false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use constants here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. The context here is very small and specific though. Anyway, feel free to make PR.

Another possibility for this code is to move it into a createPrivateCopy method in DonationComment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, actually the method name says it all.

@KaiNissen
KaiNissen merged commit 11d932a into master Jun 13, 2016
@KaiNissen
KaiNissen deleted the privcomment branch June 13, 2016 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants