Allow updating project translations / originals from GitHub #2

swissspidy · 2018-02-21T18:14:06Z

See https://github.com/wearerequired/required-network/issues/12.

Usage:

Automatically run via incoming webhook
Run wp @dev --url=https://translate.required.local traduttore update_from_github https://github.com/wearerequired/klingler-theme on the command line

This relies on the "Source file URL" field being set in a project. Example:

Todo:

Test with webhook
Remove scheduled tasks upon plugin deactivation.
Delete temporary files and folders upon plugin uninstall.
Perhaps delete temporary POT file after import.

grappler · 2018-02-23T04:19:16Z

So the idea is that whenever we push to a repo the POT file get automatically updated. We could also manually trigger the update via WP-CLI.

This though only works for plugins and themes that are in a single repo?

Where will the git repos live on the server?

swissspidy · 2018-02-23T07:19:30Z

Yes, that‘s the idea behind https://github.com/wearerequired/required-network/issues/12.

Why manually when you can do it all automatically? :-)

Yes, it only works for single theme and plugin repos. Those are the only ones we have (and should) set up in GlotPress anyway.

The Git repos currently are being cloned to a location inside /tmp/.

grappler · 2018-02-23T07:40:31Z

Yes, it only works for single theme and plugin repos. Those are the only ones we have (and should) set up in GlotPress anyway.

I am sorry to burst your bubble 😛 We have PH in Glotpress and we don't have separate repos for the themes. https://translate.required.com/projects/prohelvetia/

swissspidy · 2018-02-23T08:04:45Z

~~Yeah but all the strings are in the sub projects?~~

Edit: Scratch that. You're right.

swissspidy · 2018-02-26T08:42:10Z

@grappler regarding PH: I think it's best to move the themes into their own repository instead of trying to treat it as a special case.

grappler · 2018-02-26T08:45:17Z

I think we should be OK for PH like it is for the moment. We can open a issue and work on it in the future if need be.

Makes removing files and folders created by the plugin easier

Removes all temporary files and folder upon plugin uninstall.

…e's endpoints

Access restriction runs before that constant is defined.

Otherwise the custom events won’t get added.

swissspidy · 2018-02-26T12:50:45Z

Slack notifications now work like a charm:

The project can be found via the URL. This matches the CLI command’s behavior.

Adds ZIP URL to the Slack message for easier debugging.

swissspidy · 2018-02-26T19:55:45Z

@grappler @ocean90 If you could do a quick review before I merge this for further testing, I‘d really appreciate it.

ocean90 · 2018-02-26T20:45:31Z

README.md

+Easily update a project's translatable string from a GitHub repository.
+
+```bash
+wp traduttore update_from_github <repository_url>


I haven't seen a WP-CLI command using underscores yet. How about

wp traduttore translations build <project> to generate a ZIP file

wp traduttore translations update <repository_url> to fetch strings from a GitHub repo

I'm wondering why it has to be the repository URL for updating. Why not <project>?

I could change it so that both a URL and an ID would work. The IDs are nowhere revealed in the UI, so a URL might be easier to grasp.

Oh, I meant the project path for <project>.

ocean90 · 2018-02-26T20:48:47Z

README.md

+3. Set `https://translate.required.com/wp-json/github-webhook/v1/push-event` as the payload URL.
+4. Choose `application/json` as the content type.
+5. Enter the secret key that can be found in 1Password.
+6. Leave the other options unchanged and submit the form.


I'd be explicit here and mention that we only need the push event.

ocean90 · 2018-02-26T20:57:11Z

inc/Plugin.php

+		}
+
+		if ( ! wp_next_scheduled( 'traduttore_update_from_github', [ $params['repository']['url'] ] ) ) {
+			wp_schedule_single_event( time() + MINUTE_IN_SECONDS * 15, 'traduttore_update_from_github', [ $params['repository']['html_url'] ] );


Does it really have to be 15 minutes? 1-2 minutes should be fine too.

ocean90 · 2018-02-26T20:59:17Z

inc/Plugin.php


-			do_action( 'traduttore_zip_generated', $success, $translation_set );
+		add_action( 'traduttore_update_from_github', function ( $repository ) {
+			$project = GitHubUpdater::find_project( $repository );


There should be a check whether an update is already running. The lock can be stored as a project meta.

https://github.com/GlotPress/GlotPress-WP/blob/68bffccae4b3d3ee9971e431f6dac262c7b49086/gp-includes/meta.php

Should it return a WP_Error object when there are failures like existing locks? Would make for better error messages / logs.

Maybe? Looks like you may not be able to use them for the Slack event.

Hmm I'll leave that for the future. Not sure it's really worth it right now.

ocean90 · 2018-02-26T20:59:50Z

inc/Plugin.php

+				'action'      => 'traduttore_updated_from_github',
+				'description' => __( 'When new translations are updated from GitHub', 'traduttore' ),
+				'message'     => function( GP_Project $project, array $stats ) {
+					list($originals_added, $originals_existing, $originals_fuzzied, $originals_obsoleted, $originals_error) = $stats;


CS: Missing spaces.

ocean90 · 2018-02-26T21:08:20Z

inc/Plugin.php

+		if ( ! in_array( $event_name, [ 'push', 'ping' ], true ) ) {
+			return false;
+		}
+


Shouldn't this ignore commits to branches that are not relevant for the translations? The branch could be extracted from the source file URL.

The branch can be retrieved from the ref field. Will add a check for refs/heads/master.

ocean90 · 2018-02-26T21:12:48Z

inc/GitHubUpdater.php

+	 * @since 2.0.0
+	 *
+	 * @param string $repository GitHub repository URL, e.g. https://github.com/wearerequired/required-valencia.
+	 *


CS: No newline between @param and @return tags.

Do you perhaps know a PhpStorm setting that doesn't automatically add a newline there?

Maybe it's "Blank lines around parameters"? That's the only one I have not enabled.

grappler

Looks good otherwise.
Accessing https://translate.required.local/wp-json/github-webhook/v1/push-event in the browser gave a 404.

grappler · 2018-02-28T03:48:43Z

inc/GitHubUpdater.php

+		 * @param array      $stats        Stats about the number of imported translations.
+		 * @param PO         $translations PO object containing all the translations from the POT file.
+		 */
+		//do_action( 'traduttore_updated_from_github', $this->project, $stats, $translations );


Why is this commented out?

Whoops testing leftover :-) I didn‘t want to send tons of Slack messages locally.

swissspidy · 2018-02-28T07:32:42Z

@grappler The REST endpoint only accepts POST requests. GET will result in a 404.

ocean90 · 2018-02-28T08:54:53Z

inc/GitHubUpdater.php

+		$git_target = get_temp_dir() . 'traduttore-github-' . $slug;
+		$pot_target = wp_tempnam( 'traduttore-' . $slug . '.pot' );
+
+		if ( $this->has_lock() ) {


This should be right at the beginning of this method do avoid wp_tempnam() creating a file unnecessarily.

ocean90 · 2018-02-28T08:56:30Z

inc/GitHubUpdater.php

+
+		$stats = GP::$original->import_for_project( $this->project, $translations );
+
+		$this->remove_lock();


Should be called after the action because we don't know what functions are hooked into traduttore_updated_from_github.

ocean90 · 2018-02-28T09:01:09Z

inc/GitHubUpdater.php

+			exec( escapeshellcmd( 'git pull -q' ), $output, $status );
+			chdir( $current_dir );
+		} else {
+			exec( escapeshellcmd( sprintf( 'git clone %1$s %2$s', $source, $target ) ), $output, $status );


escapeshellarg() is missing.

escapeshellcmd( sprintf( 'git clone %1$s %2$s', escapeshellarg( $source ), escapeshellarg( $target ) ) )

ocean90 · 2018-02-28T09:01:29Z

inc/GitHubUpdater.php

+	 * @return bool True on success, false otherwise.
+	 */
+	protected function create_pot_file( $source, $target, $slug ) {
+		exec( escapeshellcmd( sprintf( 'wp i18n make-pot %1$s %2$s --slug=%3$s --domain=%3$s', $source, $target, $slug ) ), $output, $status );


escapeshellarg() is missing.

ocean90 · 2018-02-28T09:12:29Z

inc/GitHubUpdater.php

+	 * @since 2.0.0
+	 */
+	protected function remove_lock() {
+		// gp_delete_meta() is internal.


Möp. GlotPress/GlotPress#879

ocean90 · 2018-02-28T09:14:59Z

inc/Plugin.php


-			do_action( 'traduttore_zip_generated', $success, $translation_set );
+		add_action( 'traduttore_update_from_github', function ( $repository ) {
+			$project = GitHubUpdater::find_project( $repository );


Maybe? Looks like you may not be able to use them for the Slack event.

swissspidy added 3 commits February 21, 2018 19:01

Allow updating project translations from GitHub

0f9c2aa

Merge branch 'master' into update-from-github

471ec9e

Rename constant

0a73d56

swissspidy requested review from ocean90 and grappler February 21, 2018 18:14

swissspidy added 16 commits February 26, 2018 10:57

Remove POT file after import

94a07d3

Prefix POT files as well

5aaacca

Makes removing files and folders created by the plugin easier

Add plugin uninstall handler

3cb06a1

Removes all temporary files and folder upon plugin uninstall.

Clear scheduled hooks upon plugin deactivation

8fd58e6

Filter Restricted Site Access to allow external requests to Traduttor…

f530014

…e's endpoints

Remove REST_REQUEST check

e3cb0a4

Access restriction runs before that constant is defined.

Allow GitHub webhook ping requests

6c17caf

Update readme

2248b75

Fix undefined index notice

e4a8320

Use html_url field for cron event as well

162a042

Use correct URL field

7d84d37

Move action hook to GitHubUpdater class

3e1ac31

Add new Slack event for translation imports

66ad3f4

Fix Slack event descriptions

2216dc1

Load Traduttore before the Slack plugin

6487454

Otherwise the custom events won’t get added.

Improve Slack message format

81f6423

swissspidy added 2 commits February 26, 2018 14:20

Remove cache dir on uninstall too

25ae46a

Slightly adjust readme

621cba8

swissspidy added 7 commits February 26, 2018 17:38

Update slack message

def8d64

Update docs

bd13047

Rename method param

4938ae7

Only pass repository URL to cron event

255f742

The project can be found via the URL. This matches the CLI command’s behavior.

Move action to the ZIP provider.

68db79d

Adds ZIP URL to the Slack message for easier debugging.

Update docs

aaafb18

Don’t be overly specific

566c3f6

ocean90 requested changes Feb 26, 2018

View reviewed changes

swissspidy added 9 commits February 26, 2018 22:28

Reduce cron event time to 3 minutes

21384d2

Rename CLI command

aa2139b

Whitespace adjustments

8f03871

Support project paths and IDs for update command

ee2cce5

Be more explicit in the readme

7765aa6

Update commands in readme

e63d894

Webhooks: Don’t do anything for non-default branches

15b6cbb

Remove unused var

da62fb2

Add lock mechanism

f109e90

swissspidy requested a review from ocean90 February 26, 2018 22:13

grappler reviewed Feb 28, 2018

View reviewed changes

ocean90 requested changes Feb 28, 2018

View reviewed changes

swissspidy added 3 commits February 28, 2018 10:49

Uncomment action

081d382

Improve locks

7cbb39b

Escape shell args

9e0ff24

swissspidy requested a review from ocean90 February 28, 2018 10:04

ocean90 approved these changes Feb 28, 2018

View reviewed changes

swissspidy merged commit addcd52 into master Feb 28, 2018

swissspidy deleted the update-from-github branch February 28, 2018 10:42

swissspidy mentioned this pull request Apr 30, 2018

Separate customizations from API #6

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow updating project translations / originals from GitHub #2

Allow updating project translations / originals from GitHub #2

swissspidy commented Feb 21, 2018 •

edited

Loading

grappler commented Feb 23, 2018

swissspidy commented Feb 23, 2018

grappler commented Feb 23, 2018 •

edited

Loading

swissspidy commented Feb 23, 2018 •

edited

Loading

swissspidy commented Feb 26, 2018

grappler commented Feb 26, 2018

swissspidy commented Feb 26, 2018

swissspidy commented Feb 26, 2018

ocean90 Feb 26, 2018

ocean90 Feb 26, 2018

swissspidy Feb 26, 2018

ocean90 Feb 26, 2018

ocean90 Feb 26, 2018

ocean90 Feb 26, 2018

ocean90 Feb 26, 2018

swissspidy Feb 27, 2018

ocean90 Feb 28, 2018

swissspidy Feb 28, 2018

ocean90 Feb 26, 2018

ocean90 Feb 26, 2018

swissspidy Feb 26, 2018

ocean90 Feb 26, 2018

swissspidy Feb 26, 2018

ocean90 Feb 26, 2018

grappler left a comment

grappler Feb 28, 2018

swissspidy Feb 28, 2018

swissspidy commented Feb 28, 2018

ocean90 Feb 28, 2018

ocean90 Feb 28, 2018

ocean90 Feb 28, 2018

ocean90 Feb 28, 2018

ocean90 Feb 28, 2018

ocean90 Feb 28, 2018


		$stats = GP::$original->import_for_project( $this->project, $translations );

		$this->remove_lock();

Allow updating project translations / originals from GitHub #2

Allow updating project translations / originals from GitHub #2

Conversation

swissspidy commented Feb 21, 2018 • edited Loading

grappler commented Feb 23, 2018

swissspidy commented Feb 23, 2018

grappler commented Feb 23, 2018 • edited Loading

swissspidy commented Feb 23, 2018 • edited Loading

swissspidy commented Feb 26, 2018

grappler commented Feb 26, 2018

swissspidy commented Feb 26, 2018

swissspidy commented Feb 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grappler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swissspidy commented Feb 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swissspidy commented Feb 21, 2018 •

edited

Loading

grappler commented Feb 23, 2018 •

edited

Loading

swissspidy commented Feb 23, 2018 •

edited

Loading