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

Generate zip file names with the version number in them #143

Merged
merged 5 commits into from
Mar 15, 2019

Conversation

swissspidy
Copy link
Collaborator

Description
See #140.

How has this been tested?
Added more unit tests.

Types of changes
New feature (non-breaking change which adds functionality)
Kind of a breaking change as file names change.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@swissspidy swissspidy added the [Component] API Concerns REST API Endpoints, e.g. for incoming webhooks label Dec 20, 2018
@codecov-io
Copy link

codecov-io commented Dec 20, 2018

Codecov Report

Merging #143 into master will decrease coverage by 0.16%.
The diff coverage is 67.34%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #143      +/-   ##
============================================
- Coverage     79.79%   79.63%   -0.17%     
- Complexity      379      397      +18     
============================================
  Files            24       24              
  Lines           896      928      +32     
============================================
+ Hits            715      739      +24     
- Misses          181      189       +8
Impacted Files Coverage Δ Complexity Δ
inc/Project.php 100% <ø> (ø) 53 <0> (ø) ⬇️
inc/ZipProvider.php 98.82% <100%> (+0.12%) 25 <0> (+1) ⬆️
inc/Plugin.php 41.76% <55.55%> (+4.09%) 71 <7> (+17) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba2340f...7de4181. Read the comment docs.

@swissspidy swissspidy requested a review from ocean90 December 21, 2018 10:20
grappler
grappler previously approved these changes Mar 14, 2019
Copy link
Member

@ocean90 ocean90 left a comment

Choose a reason for hiding this comment

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

I think we have to force a new build when a version changes without any string changes.

This allows zip generation to be scheduled even if the translations have not be changed but some other parameter related to the zip file.
The package version can change without any changes with the strings
inc/Plugin.php Outdated
add_filter(
'gp_update_meta',
function( $meta_tuple ) {
$project = ( new ProjectLocator( $meta_tuple['object_id'] ) )->get_project();
Copy link
Member

Choose a reason for hiding this comment

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

The check for the $allowed_keys should come before checking for the project as it seems to be less expensive.

@grappler
Copy link
Contributor

grappler commented Mar 14, 2019

Once the next version of Glotpress is released the code could be changed to

diff --git a/inc/Plugin.php b/inc/Plugin.php
index 3c6af31..1fcd9d8 100644
--- a/inc/Plugin.php
+++ b/inc/Plugin.php
@@ -53,7 +53,12 @@ class Plugin {

                add_action(
                        'gp_translation_saved',
-                       function ( GP_Translation $translation ) {
+                       function ( GP_Translation $translation, GP_Translation $translation_before ) {
+                               // Only run if translation is added or removed from current.
+                               if ( 'current' !== $translation->status && 'current' !== $translation_before->status ) {
+                                       return;
+                               }
+
                                /* @var GP_Translation_Set $translation_set */
                                $translation_set = GP::$translation_set->get( $translation->translation_set_id );

@@ -62,18 +67,11 @@ class Plugin {
                                        return;
                                }

-                               $last_modified = $translation_set->last_modified();
-                               if ( $last_modified ) {
-                                       $last_modified = new DateTime( $last_modified, new DateTimeZone( 'UTC' ) );
-                               } else {
-                                       $last_modified = new DateTime( 'now', new DateTimeZone( 'UTC' ) );
-                               }
-
                                $zip_provider = new ZipProvider( $translation_set );
-                               if ( $last_modified > $zip_provider->get_last_build_time() ) {
-                                       $zip_provider->schedule_generation();
-                               }
-                       }
+                               $zip_provider->schedule_generation();
+                       },
+                       10,
+                       2
                );

@grappler grappler merged commit 3db35e9 into master Mar 15, 2019
@delete-merged-branch delete-merged-branch bot deleted the versioned-zips branch March 15, 2019 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] API Concerns REST API Endpoints, e.g. for incoming webhooks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants