Skip to content

Conversation

@dd32
Copy link
Contributor

@dd32 dd32 commented Sep 19, 2022

What?

As a follow up to #1451, it looks like the action POST variable was looked at incorrectly, causing a PHP Notice:

E_NOTICE: Undefined variable: new_status in wp-content/plugins/glotpress/gp-includes/routes/translation.php:438

Based on https://github.com/amieiro/GlotPress-WP/blob/develop/gp-includes/routes/translation.php#L375-L381 it looks like the correct is reject, which is confirmed from the POST data attached to the above notice containing 'reject'.

I believe this is potentially causing bulk rejections to fail, based on re-attempts after failure.

Additionally, $new_status should probably be pre-set to rejected before the switch.

Why?

How?

Testing Instructions

Screenshots or screencast

@dd32 dd32 added the [Type] Bug An existing feature is broken. label Sep 19, 2022
@dd32 dd32 requested a review from amieiro September 19, 2022 04:48
@dd32 dd32 changed the title Use the correct action name Bulk status change: Fix bulk rejection Sep 19, 2022
@dd32
Copy link
Contributor Author

dd32 commented Sep 19, 2022

It appears this is probably the underlying cause of https://meta.trac.wordpress.org/ticket/6488 as it'll have set the Database rows to have an empty translation status field.

Copy link
Member

@amieiro amieiro left a comment

Choose a reason for hiding this comment

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

I have modified another lines, because we have the same error in the "_bulk_approve" function.

@amieiro amieiro merged commit 1022594 into develop Sep 19, 2022
@amieiro amieiro deleted the fix/1451-reject branch September 19, 2022 12:02
@NekoJonez
Copy link
Contributor

Related: https://meta.trac.wordpress.org/ticket/6488

@ocean90 ocean90 added this to the 4.0 milestone Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature is broken.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants