Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unlimited comment/post editing #2826

Merged
merged 10 commits into from Oct 24, 2018
Merged

unlimited comment/post editing #2826

merged 10 commits into from Oct 24, 2018

Conversation

roadscape
Copy link
Contributor

Minor cleanup of related fields and removal of comment-editing-after-payout restriction. Untested! Do not merge until majority witness upgrade to a compatible version is confirmed!

@roadscape roadscape changed the title Unlimited editing unlimited comment/post editing May 29, 2018
@roadscape
Copy link
Contributor Author

Witnesses don't seem to be ready yet. Closing for now, set a reminder to check back in 2 weeks.

@roadscape roadscape closed this Jun 12, 2018
@roadscape roadscape reopened this Jul 10, 2018
@roadscape
Copy link
Contributor Author

Last time I tested this branch, I received assert errors regarding changing permlink. Not sure if this was due to condenser logic or steemd pre-softfork-removal.

@roadscape
Copy link
Contributor Author

Rebased to resolve conflicts, but appears as though adoption of 0.19.10 is still a few weeks away. Checking back in a few weeks.

@roadscape roadscape closed this Jul 12, 2018
@creatorguy
Copy link

To all involved,

We are delighted and excited to see this change on the verge of being released to the world. I must confess that we have been waiting very impatiently, having been lobbying for this change literally for years now...

And so, I'm just popping in here today to say both "Hooray!" and to, if possible, gently appeal for adoption and release of this feature as soon as practicably possible.

Thanks so much in advance!

@TimCliff
Copy link
Contributor

@creatorguy it is mainly a matter of enough witnesses applying the 19.10 changes. Currently only 5 of the top 20 have applied them. You can see the list here: https://steemd.com/witnesses. Once more witnesses have applied the changes, then this change will be taken off hold.

@creatorguy
Copy link

creatorguy commented Jul 24, 2018

Hello, @TimCliff,

I really appreciate your reply and further explanation of what's holding this update back. Now I can use whatever influence I may have, especially with the witnesses that I support with my vote, to encourage adoption of these changes.

Thanks! :D

P.S. I would appreciate it if you could tell me exactly how many witnesses must apply 19.10 before the change is implemented? All of the top 20? Some percentage of all witnesses? Thanks in advance.

P.P.S. Simply as a gesture of my appreciation for your kind feedback already given, I discovered that I had a poorly used witness vote, and am now supporting your witness. ;)

@genelamarellis
Copy link

Hey @roadscape,
Question. Will this allow editing of existing posts/comments, or just new posts/comments from the time 19.10 is implemented?

Also, has this been successfully implemented? From looking at the Steem Witness list (https://steemd.com/witnesses), it seems that some are already running 19.10. How many more need to be running this for it to be activated, and any idea when that will happen?

Thanks!

@syvb
Copy link
Contributor

syvb commented Aug 7, 2018

@genelamarellis This feature already works, it just hasn't been enabled on Steemit.com. With the number of upgraded witnesses/nodes, updating posts after 7 days works with some consistency. (the edits usually make it through the network, sometimes you need to try a few times). This would allow editing all posts ever made.

@genelamarellis
Copy link

Oh I see. Got it. Thank you!

@roadscape roadscape reopened this Sep 20, 2018
@ajayyy
Copy link

ajayyy commented Sep 27, 2018

Is this still happening?

@syvb
Copy link
Contributor

syvb commented Sep 27, 2018

@ajayyy Yep. Now that all witnesses are running AppBase, it should be ready to merge.

@roadscape
Copy link
Contributor Author

This was tested recently and resulted in the same assertion error (cannot change permlink), need to find the cause.

@roadscape
Copy link
Contributor Author

Now, ReferenceError: formId is not defined. Perhaps a new reference to it was added since this branch removed it 4 months ago.

@eonwarped
Copy link
Contributor

eonwarped commented Oct 2, 2018 via email

@@ -324,7 +324,6 @@ class ReplyEditor extends React.Component {
reply,
username,
isStory,
formId,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only line you need to revert to get it to work.

@eonwarped
Copy link
Contributor

#3060 available as well. @roadscape

@eonwarped
Copy link
Contributor

Also question, should categories be editable? Seems like it's allowed, but just wanted to double-check.

@relativityboy
Copy link
Contributor

relativityboy commented Oct 17, 2018

@eonwarped unless something changed in HF20, the category is set when the post is created, and is unalterable.

@eonwarped
Copy link
Contributor

@relativityboy Ah I meant other tags. The first one I didn't check but I changed the others.

@@ -300,11 +300,11 @@ class CommentImpl extends React.Component {
// hide images if author is in blacklist
const hideImages = ImageUserBlockList.includes(author);

const showDeleteOption = username === author && allowDelete;
const _isPaidout = comment.cashout_time === '1969-12-31T23:59:59'; // TODO: audit after HF19. #1259
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment here can probably be removed. We're post HF20 and the time hasn't been changed.

Were it within scope, that string '1969-12-31T23:59:59' should be hoisted as a const and kept somewhere with good documentation.

relativityboy
relativityboy previously approved these changes Oct 22, 2018
Copy link
Contributor

@relativityboy relativityboy left a comment

Choose a reason for hiding this comment

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

LGTM.

@roadscape roadscape removed the blocked label Oct 22, 2018
@relativityboy relativityboy dismissed their stale review October 23, 2018 21:32

Dismissing my own review. I'd thought this code had been tested in dev before the PR was created.

@relativityboy
Copy link
Contributor

Needs more work

Testing in dev I was not able to update an old post. I have found 3 issues. The first two may be related to the environment, and the third may be related to steemd.

  1. When hard-landing on a Post page (something one could easily do if one bookmarked the post for later editing) I get the following in the console: JS Error
  2. Sometimes, when I click the update button, a spinner replaces the button and the edit boxes gray-out, but no RPC call is sent. Spinner and boxes do not revert or update again.
  3. When an RPC call has gone out after clicking update I get the following error Assert Exception:equal( com.parent_permlink, o.parent_permlink ): The permlink of a comment cannot change. The value of the permlink in the RPC call looks correct (based on checking the value from the original get_state call), and looks correct in the stack-trace send back by steemd. I've reproduced this with two different posts (same author). At first blush - this doesn't look like condenser.

Details of the last item
get_state
get_state response

Call to ["network_broadcast_api","broadcast_transaction_synchronous"]:

{"id":14,"jsonrpc":"2.0","method":"call","params":["network_broadcast_api","broadcast_transaction_synchronous",[{"ref_block_num":3797,"ref_block_prefix":565546026,"expiration":"2018-10-23T21:39:03","operations":[["comment",{"parent_author":"","parent_permlink":"","author":"test-safari","permlink":"tester","title":"Tester","body":"Well, doing it again... editing this old post...","json_metadata":"{\"app\":\"steemit/0.1\",\"format\":\"markdown\",\"tags\":[\"test\"]}"}]],"extensions":[],"signatures":["1f60cab8bf09098723cb6f45ec0c211f4cb34a0c7e6bd880824737fb9294c6bf7a68383afe48ab02e45ab34fa3b3bcbf3aad47e5ffa221ff2cdc24d1b69af58f7c"]}]]}

Screen of the response:
screen shot 2018-10-23 at 4 36 30 pm

Src of the response:

{"jsonrpc":"2.0","error":{"code":-32000,"message":"Assert Exception:equal( com.parent_permlink, o.parent_permlink ): The permlink of a comment cannot change.","data":{"code":10,"name":"assert_exception","message":"Assert Exception","stack":[{"context":{"level":"error","file":"steem_evaluator.cpp","line":863,"method":"operator()","hostname":"","timestamp":"2018-10-23T21:29:05"},"format":"equal( com.parent_permlink, o.parent_permlink ): The permlink of a comment cannot change.","data":{}},{"context":{"level":"warn","file":"steem_evaluator.cpp","line":904,"method":"do_apply","hostname":"","timestamp":"2018-10-23T21:29:05"},"format":"","data":{"o":{"parent_author":"","parent_permlink":"","author":"test-safari","permlink":"tester","title":"Tester","body":"Well, doing it again... editing this old post...","json_metadata":"{\"app\":\"steemit\/0.1\",\"format\":\"markdown\",\"tags\":[\"test\"]}"}}},{"context":{"level":"warn","file":"database.cpp","line":3485,"method":"_apply_transaction","hostname":"","timestamp":"2018-10-23T21:29:05"},"format":"","data":{"op":{"type":"comment_operation","value":{"parent_author":"","parent_permlink":"","author":"test-safari","permlink":"tester","title":"Tester","body":"Well, doing it again... editing this old post...","json_metadata":"{\"app\":\"steemit\/0.1\",\"format\":\"markdown\",\"tags\":[\"test\"]}"}}}},{"context":{"level":"warn","file":"database.cpp","line":3491,"method":"_apply_transaction","hostname":"","timestamp":"2018-10-23T21:29:05"},"format":"","data":{"trx":{"ref_block_num":3797,"ref_block_prefix":565546026,"expiration":"2018-10-23T21:39:03","operations":[{"type":"comment_operation","value":{"parent_author":"","parent_permlink":"","author":"test-safari","permlink":"tester","title":"Tester","body":"Well, doing it again... editing this old post...","json_metadata":"{\"app\":\"steemit\/0.1\",\"format\":\"markdown\",\"tags\":[\"test\"]}"}}],"extensions":[],"signatures":["1f60cab8bf09098723cb6f45ec0c211f4cb34a0c7e6bd880824737fb9294c6bf7a68383afe48ab02e45ab34fa3b3bcbf3aad47e5ffa221ff2cdc24d1b69af58f7c"]}}},{"context":{"level":"warn","file":"database.cpp","line":817,"method":"push_transaction","hostname":"","timestamp":"2018-10-23T21:29:05"},"format":"","data":{"trx":{"ref_block_num":3797,"ref_block_prefix":565546026,"expiration":"2018-10-23T21:39:03","operations":[{"type":"comment_operation","value":{"parent_author":"","parent_permlink":"","author":"test-safari","permlink":"tester","title":"Tester","body":"Well, doing it again... editing this old post...","json_metadata":"{\"app\":\"steemit\/0.1\",\"format\":\"markdown\",\"tags\":[\"test\"]}"}}],"extensions":[],"signatures":["1f60cab8bf09098723cb6f45ec0c211f4cb34a0c7e6bd880824737fb9294c6bf7a68383afe48ab02e45ab34fa3b3bcbf3aad47e5ffa221ff2cdc24d1b69af58f7c"]}}}]}},"id":14}

@relativityboy
Copy link
Contributor

Ok. After testing in other environments, looks like the issue may have been the dev env. We should be gtg.

@relativityboy relativityboy merged commit 22e6518 into master Oct 24, 2018
@roadscape roadscape deleted the unlimited-editing branch December 20, 2018 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants