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

Add a new status: changes requested #1451

Merged
merged 27 commits into from
Sep 15, 2022

Conversation

amieiro
Copy link
Member

@amieiro amieiro commented Jun 14, 2022

Problem

This PR, with another one in the gp-translation-helpers plugin, adds a new status: changes requested.

Solution

GlotPress stores a new status (changes_requested) when:

  • A simple rejection is made, adding some feedback.
  • A bulk rejection is made, adding some feedback.

Testing instructions

To test this PR, you have to use the related branch from gp-translation-helpers plugin and you have to reject some strings as validator (simple or bulk rejection), adding some feedback.

Screenshots or screencast

The new status in the main table:

image

The new status in a string:

image

Rejecting a string:

image

When a validator adds feedback (reason and/or comment), the "Reject" button becomes "Request changes", so the validator sets the string in this new status. A notification is sent to the translator.

image

Making a bulk reject:

image

When a validator adds feedback (reason and/or comment), the "Reject" button becomes "Request changes", so the validator sets the string in this new status. A notification is sent to the translator.

image

The new status in the filters:

image

@amieiro amieiro marked this pull request as draft June 15, 2022 11:26
amieiro added 2 commits June 16, 2022 13:42
…BLOGID constant available

To view the new status in the frontend, you need to add the WPORG_TRANSLATE_BLOGID
constant in the wp-config.php file with the value of the get_current_blog_id() function. E.g.:

   define( 'WPORG_TRANSLATE_BLOGID', 1 );
…lled

Check if the 'gp_translation_helper_installed' filter returns true -> plugin installed.
Copy link
Member

@akirk akirk left a comment

Choose a reason for hiding this comment

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

I reviewed this from a code perspective and something about the underscore makes it seem off. Could we rename changes_requested to something without underscore or hyphen. Maybe justchangesrequested. Or changerequest or reviewed?

@amieiro amieiro marked this pull request as ready for review August 9, 2022 11:43
@amieiro amieiro requested a review from akirk August 9, 2022 11:43
Copy link
Member

@akirk akirk left a comment

Choose a reason for hiding this comment

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

The JS error needs to be fixed because it breaks much of the GlotPress UI.

Copy link
Member

@akirk akirk left a comment

Choose a reason for hiding this comment

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

I've tested that the PR doesn't change the behavior of GlotPress unless I use it in combination with GlotPress/gp-translation-helpers#65, which is exactly what we want.

@amieiro amieiro merged commit 4b7ddac into GlotPress:develop Sep 15, 2022
@ocean90 ocean90 added this to the 4.0 milestone Oct 10, 2022
@ocean90 ocean90 added the [Type] Feature New feature to highlight in changelog. label Oct 10, 2022
samuelsidler added a commit to samuelsidler/GlotPress that referenced this pull request Aug 18, 2023
Buttons margins appear to have broken after GlotPress#1451 and are now misaligned. This adjusts them back to the pre-1451 state.

Fixes GlotPress#1676.
@samuelsidler samuelsidler mentioned this pull request Aug 18, 2023
amieiro pushed a commit that referenced this pull request Aug 21, 2023
Buttons margins appear to have broken after #1451 and are now misaligned. This adjusts them back to the pre-1451 state.

Fixes #1676.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Feature New feature to highlight in changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants