WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#33644 closed defect (bug) (fixed)

Merge similar strings - deleting posts

Reported by: pavelevap Owned by: SergeyBiryukov
Milestone: 4.4 Priority: normal
Severity: normal Version: 2.5
Component: I18N Keywords: needs-patch
Focuses: Cc:

Description

There are some very similar strings:

"Sorry, you are not allowed to delete this post."
"Sorry, you do not have the right to delete this post."

We have also "You are not allowed to delete this post.", but it could be handled here: #31727

"The post cannot be deleted."
"For some strange yet very annoying reason, this post could not be deleted."

We can merge them, I guess. See attached patch.

Attachments (2)

33644.patch (978 bytes) - added by pavelevap 5 years ago.
33644.1.patch (607 bytes) - added by Ankit K Gupta 5 years ago.
Updated patch file

Download all attachments as: .zip

Change History (12)

@pavelevap
5 years ago

#1 @dreamyourcareer
5 years ago

  • Resolution set to invalid
  • Severity changed from normal to minor
  • Status changed from new to closed

@pavelevap You are correct on merging these strings, these will make the code a bit smaller.

As you know these are very similar strings. But, each string is associated with different issues merging will not help a lot for this. If you will think about troubleshooting and error handling. These strings help a lot in finding exact issues ASAP. But, if we will merge these strings it will be hard to find where the issue exactly is. Will increase time for troubleshooting for a newbie.

Our aim at WordPress is to keep it as simple as we can. So, a normal guy can operate well.

#2 @pavelevap
5 years ago

  • Resolution invalid deleted
  • Severity changed from minor to normal
  • Status changed from closed to reopened

No, it is the same issue, I guess, and that is why I suggested to merge them (I found them during localization work). First is related to missing delete_post capability, second is checking false for wp_delete_post().

#3 @netweb
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

@pavelapp, Can you refresh the patch and make all 3: Sorry, you are not allowed to delete this post.

(The 3rd one above is the one from #31727 and I think this string is better here than the change to item in 31727)

The other change to The post cannot be deleted. also makes sense

#4 @netweb
5 years ago

  • Version changed from trunk to 2.5

@Ankit K Gupta
5 years ago

Updated patch file

#5 @Ankit K Gupta
5 years ago

ignore above attached patch file, My mistake.

#6 follow-up: @pavelevap
5 years ago

@netweb: Third occurence of similar string is a little bit different and that is why I did not add it here.

There are several "You are not allowed to delete this item." strings related to post, but only once "You are not allowed to delete this post." (in the same situation). So, I preserved this pattern and only merged it in #31727 in this way. General word item is probably used because of possibility of different post types and not only post.

In this case I am following pattern of other common strings "Sorry, you do not have the right to..." and this ticket was meant only as a first step to simply kill 2 not needed strings :-)

#7 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to SergeyBiryukov
  • Status changed from reopened to reviewing

#8 in reply to: ↑ 6 @netweb
5 years ago

@pavelevap, Right, preserving patterns is important, I'll leave the review to Sergey :)

By the way, thanks again for doing these, we may not say it enough via the #polyglots team, so thank you :)

This ticket was mentioned in Slack in #core by sergey. View the logs.


5 years ago

#10 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 34798:

Merge some strings with the same meaning in wp-includes/class-wp-xmlrpc-server.php.

Props pavelevap.
Fixes #33644.

Note: See TracTickets for help on using tickets.