Making WordPress.org

Opened 5 years ago

Last modified 4 years ago

#4237 reviewing defect (bug)

Rejected plugins show wrong last_updated date

Reported by: ipstenu's profile Ipstenu Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Priority: normal
Component: Plugin Directory Keywords: has-patch
Cc:

Description

When plugin reviews aren't replied to within 6 months, we reject them. Previously, when a plugin was rejected its 'publish' date would be the date of the rejection, allowing us to calculate how many plugins were closed in a time period.

Currently when a plugin is rejected, the 'publish' date is the date it was submitted.

This change needs to be reverted to allow for proper tracking of when plugins are rejected.

I propose the following:

1) a meta value for 'submission_date' be added
2) any status change (rejected, closed, disabled, opened, etc) changes the 'last_updated' value

That would allow us to run more complex searches, in addition to properly tracking changes.

Attachments (2)

4237.diff (2.0 KB) - added by ck3lee 5 years ago.
4237.1.diff (1.5 KB) - added by ck3lee 5 years ago.

Download all attachments as: .zip

Change History (14)

#1 @coffee2code
5 years ago

  • Component changed from General to Plugin Directory

This ticket was mentioned in Slack in #meta by tellyworth. View the logs.


5 years ago

#3 @tellyworth
5 years ago

  • Owner set to ck3lee
  • Status changed from new to assigned

@ck3lee
5 years ago

#4 @ck3lee
5 years ago

  • Keywords has-patch added

Re: @Ipstenu's suggestions:

1) a meta value for 'submission_date' be added
2) any status change (rejected, closed, disabled, opened, etc) changes the 'last_updated' value

Current state:

  • Looks like we have an existing meta value _submitted_date that records the timestamp when plugin.zip is uploaded. So, we can use that for search and reports.
  • However, _submitted_date is not inserted when the plugin is imported through cli.
  • And it looks like we are also record the timestamp every time the status changes. Eg: _new, _pending, _rejected, _approved.
  • However, the timestamps are set to post_modified_gmt. And I'm not sure whether that is correct. See similar comments on related ticket: #3511 post_modified_gmt itself does not get updated when the status changes

Proposed changes:

  • When status changes, always update the current timestamp with time() instead of post_modified_gmt.
  • When status changes, always update the last_updated meta value with the current timestamp using time().
  • When plugin is imported, set _submitted_date.

#5 @Ipstenu
5 years ago

However, _submitted_date is not inserted when the plugin is imported through cli.

No one uses that though, except probably @otto42 and not even then.

When status changes, always update the last_updated meta value with the current timestamp using time().

I don't think this would work in the long run, since closing a plugin would change last_updated, and then we can't do a quick check when someone says they updated SVN but they didn't (seriously, it's a bigger problem than you might imagine, at least twice a week I have to explain that 'update SVN' means 'update SVN').

The following status changes are 'safe' to update last_updated:

  • _approved
  • _rejected

That's really it.

@ck3lee
5 years ago

#6 @ck3lee
5 years ago

Thank you for your clarification, @Ipstenu! Hope this works.

With attachment:4237.1.diff:

  • When status changes, always update the current timestamp with time() instead of post_modified_gmt.
  • When status changes to approved or rejected, update the last_updated meta value with the current timestamp using time().
  • When plugin is imported, set _submitted_date.

#7 follow-up: @Ipstenu
5 years ago

What's the reason for having this:

update_post_meta( $post->ID, "last_updated", gmdate( 'Y-m-d H:i:s', time() ) );

if you're using time() in this:

 update_post_meta( $post->ID, "_{$new_status}", time() );

Seemed odd to have them different?

Code looks fine to me though.

#8 in reply to: ↑ 7 @ck3lee
5 years ago

Replying to Ipstenu:

What's the reason for having this:

update_post_meta( $post->ID, "last_updated", gmdate( 'Y-m-d H:i:s', time() ) );

Yeah, I'm not sure why, but the format for last_updated is different than the rest. So, this keeps the last_updated date format the same.

This ticket was mentioned in Slack in #meta by ipstenu. View the logs.


4 years ago

This ticket was mentioned in Slack in #meta by ipstenu. View the logs.


4 years ago

#11 @tellyworth
4 years ago

  • Owner changed from ck3lee to tellyworth
  • Status changed from assigned to reviewing

#12 @tellyworth
4 years ago

  • Owner changed from tellyworth to SergeyBiryukov
Note: See TracTickets for help on using tickets.