Do you handle translations via po files and then import them to Drupal?

This module will help you to have reports on your po translations status.

Configure the path to a folder containing po files and you'll get some reports
on them.

Reports are made for the number of translated strings, number of untranstaled
strings, number of strings that are translatad but contain some non allowed
HTML (thus you will not be able to import it to the database), number of total
strings per a po file.

Checking that translations do not contain non allowed HTML uses the same
verifications as Local module. This way, no surprises will occur when
importing po files with Local module.

A last row adds the totals for all po files in the folder.

Note that results are displayed in a table that is sortable by any column you
want. This uses Drupal Core tablesort.

This module does not add any table to the database nor does it relay on the
database for its reports. It only does on fly reports reading po files.

Requirements

Local module (Drupal Core module) is required to reuse the same checks for HTML Allowness.

Configuration

Make sure to have 'access administration pages' permission to be able to
configure the module at /po_translations_report/settings/PoTranslationsReportAdmin

Make sur to have 'access po translations report' permission to be able to access
reports at /po_translations_report

Tests

This module implements some functionnal tests that have the group name
'Po Translations Report'

Project link:

Git:

Drupal version:

  • 8.x

Reviews:

  1. [D7] Hash Access
    https://drupal.org/node/2208357#comment-8781609
  2. [D7] OPAC Search
    https://drupal.org/node/2257061#comment-8759733
  3. [D7] Credly
    https://drupal.org/node/2254303#comment-8751397
    https://drupal.org/node/2254303#comment-8755805
    https://drupal.org/node/2254303#comment-8781527
  4. [D7] Semantic UI
    https://drupal.org/node/2211003#comment-8751205
  5. [D7] Biblio Search Api
    https://drupal.org/node/2150831#comment-8654673
  6. [D7] Clean up translations
    https://drupal.org/node/2173743#comment-8654645
  7. [D7] race_time
    https://drupal.org/node/2163753#comment-8654551
  8. [D7] Site Alerts
    https://drupal.org/node/222697#comment-8654497
  9. [D7] Missing Module
    https://drupal.org/node/1948458#comment-7352368
  10. [D7] Admin Users
    https://drupal.org/node/1959320#comment-7352460
    https://drupal.org/node/1959320#comment-7732627
    https://drupal.org/node/1959320#comment-7736847
  11. [D7] Taxonomy Extends
    https://drupal.org/node/1917832#comment-7352426
CommentFileSizeAuthor
#13 coder-results.txt3.03 KBklausi

Comments

izus’s picture

izus commented
Issue summary: View changes
izus’s picture

izus commented
Issue summary: View changes
PA robot’s picture

PA robot commented
Status: Active » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxizus2250391git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

izus’s picture

izus commented
Issue tags: +PAreview: review bonus

adding review bonus tag

izus’s picture

izus commented
Status: Needs work » Needs review

Hi,
i fixed code documentation and advises from pareview.sh
it is ready for review now.
Thanks

izus’s picture

izus commented
Issue summary: View changes

added more code reviews

izus’s picture

izus commented
Issue summary: View changes

Updating project reviews list

izus’s picture

izus commented
Issue summary: View changes

updating reviews list

izus’s picture

izus commented
Priority: Normal » Major

Updating Priority as mentioned in https://drupal.org/node/894256#anchor-workflow

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Needs work
Duplication
This sounds like a feature that should live in the existing tmgmt project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the tmgmt issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason or I've wildly misinterpreted the purpose of the project please get back to us and set this back to "needs review".

izus’s picture

izus commented
Status: Needs work » Needs review

hi,
actually tmgmt project cares of translations itself and translation sources as Microsoft's translation service or Google, it also deals with translation workflow in general and having the translations in database for content and interface.

Po Translations Report does not deal with translating content nor interface :) it doesn't deal at all with translations in the database, it never translates anything ! All what it does is having translation reports from po files and for this it uses the Drupal 8 Core API classes and methods to generate reports. (actually at first, i started with a thrid library for parsing po files but i looked again at the new shiny Core and it really has all what's needed decoupled (that wasn't the case for D7) so Thanks Drupal 8 :)

So this is a great difference, actually for interface translations, sometimes there is a lot to translate that it's better to let this part to a third party, The module just scans the po's to have stats, display them and warn if there are strings that are not safe and will never be accepted by Drupal core locale module.

Thanks

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented

Hmm. It seems like a feature that should be in tmgmt though. But i'll take your word for it. However, I feel more comfortable if you posted a question in tmgmt's issue queue requesting confirmation that they do not intend to provide this type of functionality.

Review of the code: that's going to be difficult. Part of the review process is to make sure there is a firm grasp of how Drupal's core API and its primary contrib modules are used. Considering that there are still changes to D8 and there are very few contrib modules ready yet, this is hard to do. Overall, the project looks OK. But I feel like you're going to need someone from the i18n initiative to look at it.

Leaving as needs review for now.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Priority: Major » Normal
Status: Needs review » Needs work
StatusFileSize
new coder-results.txt3.03 KB

Review of the 8.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. po_translations_report.routing.yml:/po_translations_report/settings/PoTranslationsReportAdmin: access administration vs. administer site configuration: The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.
  2. Your are still using PSR-0 for your classes, you can switch to PSR-4 now to reduce the number of nested directories.
  3. PoTranslationsReportController: do not use \Drupal in classes - you must use dependency injection to get the config service for example. See https://drupal.org/node/2133171
  4. PoTranslationsReportController: all class variables need to be documented. $notAllowedTranslationCount is surely not self-explaining.

So the wrong permission is a blocker right now, otherwise looks good to me.

izus’s picture

izus commented
Status: Needs work » Needs review

hi,
Thanks for the review.
I changed the permission name and switched to PSR-4 as Core did (Tests are green ! YAY !)
I documented the variables that are missing a documentation.
I will try later for dependency injection of the configuration as i didn't succed for this at the first glance.
So here it is ready for a review again :)

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: Unassigned » mpdonadio
Status: Needs review » Reviewed & tested by the community

manual review:

  1. could you also fix the things pointed out by Coder (missing method visibility etc.)?
  2. PoTranslationsReportController: "catch (\Exception $exception) { throw $exception; }": that does not make sense, why do you need to catch the exception if you are going to rethrow it immediately anyway?

But otherwise looks good to me.

Assigning to mpdonadio as he might have time to take a final look at this.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Assigned: mpdonadio » Unassigned

I will admit that my Drupal 8 skills are behind right now, and I imploded my test site so I can't run the tests right now.

It looks like the online pareview.sh needs to be updated (I let @patrickd_de know). The Coder issues that @klausi mentions are 


FILE: /home/matt/tmp/pareview_temp/po_translations_report.module
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 30 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------


FILE: /home/matt/tmp/pareview_temp/src/Tests/PoTranslationsReportTest.php
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
 40 | ERROR | Visibility must be declared on method
    |       | "testPoTranslationsReportResults"
 56 | ERROR | Visibility must be declared on method
    |       | "testPoTranslationsReportAdminForm"
 72 | ERROR | Visibility must be declared on method "userCreateAndLogin"
 80 | ERROR | Visibility must be declared on method "getDataPath"
 89 | ERROR | Visibility must be declared on method "getDefaultHTMLResults"
--------------------------------------------------------------------------------

The permission change to 'administer site configuration' needs to be reflected in the README.

The DI is still missing from PoTranslationsReportController::content(), but as mentioned above is not a blocker.

Tests look like valid coverage.

Otherwise, OK.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, izus !

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

izus’s picture

izus commented

Yay
thanks everybody !
i updated the code to take into account #15 and #16
will work on DI and all will be good :)
Thanks again !

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

PA robot’s picture

PA robot commented
Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2258283

Project 2: https://www.drupal.org/node/2035067

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.