Comments

berdir’s picture

Title: Update Nativy Translator to new API » Finishing Translator: Nativy
berdir’s picture

Status: Active » Needs review
StatusFileSize
new54.53 KB

Ok, here is a first patch.

- Based on a completly new RESTful API.
- Support for detecting supporter target languages
- Select from 3 offers
- Redirect through manual payment page. Exact process can be improved, currently displays a message with a link.
- Implementation of callback URL that retrieves the translated order
- checkout information that displays the checkout information (currently the only thing displayed is the status
- Patch is huge due to the removal of a few files related to the old SOAP interface

ToDo (Mostly for follow-up issues)
- The redirect back to the site after doing the payment is currently broken, I contacted them about this and it looks like they started to update it but it's not documented yet how it works now. So you need to go back to the site manually.
- There is currently no possibility to configure anything on the checkout screen, e.g. those sliders or that you want a reviewer. Same for default options for those settings.

berdir’s picture

StatusFileSize
new58.63 KB

Update.

- Appends the return_url to the payment link. Works correctly now with the redirect destination, but not yet if you have multiple jobs to check out, because there is currently no way to get the next url without removing it from the queue. Will open a follow-up issue for that.
- Verifies callback url's with the now provided api_sign and timestamp.
- Added logging of requests ( to check performance of their service)

I'll probably commit this soon and then start some follow-ups, like:

- The queue redirect stuff
- Checkout options like description texts and reviewers
- We have 4 translators now that do http requests and more are coming (e.g. the one for our own server, google, maybe lingotek). They currently have some overlapping stuff that we could unify in a DefaultRemoteTranslatatorController. For example a unified request() method that automatically supports mock/test/live remote URL's and logging. Or maybe a generic hook_menu() implementation for callbacks, that forward to a method on the controller.

berdir’s picture

Title: Finishing Translator: Nativy » Update nativy translator to new API
Component: Code » Translator: Nativy
Assigned: Unassigned » berdir

Status: Needs review » Needs work

The last submitted patch, nativy_new_api2.patch, failed testing.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new60.14 KB

Updated patch that adds support for descriptions and corrector setting.

berdir’s picture

What's still missing are the settings on the right side of https://www.nativy.com/client/createorder.aspx. I think for a first version, it's enough if we implement them as simple selects. According to the documentation ( https://www.nativy.com/connectdoc/), the allowed values range between 1 and 80.

The user interface could probably also be improved a bit, by e.g. placing the translator image and the price information side by side.

And, there are currently no tests. Have a look at the mygengo/supertext/microsoft tests. There is a mock test module for each that returns example responses and the plugin has a setting that allows to override the service URL. Addiionally to what they are doing, tests for nativy should include: a) Saving the submitted order in a variable and confirm that the configured options have been submitted and b) return with/without corrector information depending on the submitted options.

berdir’s picture

Assigned: berdir » corvus_ch
corvus_ch’s picture

StatusFileSize
new63.88 KB
berdir’s picture

Status: Needs review » Needs work
+++ b/translators/nativy/tmgmt_nativy.installundefined
@@ -0,0 +1,51 @@
+ * Implements hook_schema().
+ */
+function tmgmt_nativy_schema() {
+  $schema['tmgmt_nativy_log'] = array(
+    'description' => 'Logs nativy requests',

I'm currently not sure if we should make this a generic table (in a follow up), or throw it out. Any relevant information should be added as a job message. It can be helpful for debugging, though.

+++ b/translators/nativy/tmgmt_nativy.ui.incundefined
@@ -14,18 +14,251 @@ class TMGMTNativyTranslatorUIController extends TMGMTDefaultTranslatorUIControll
 
+  protected function getJobSetting(TMGMTJob $job, $setting, $default = '') {
+    return isset($job->settings[$setting]) ? $job->settings[$setting] : $default;

Needs a docblock.

+++ b/translators/nativy/tmgmt_nativy.ui.incundefined
@@ -14,18 +14,251 @@ class TMGMTNativyTranslatorUIController extends TMGMTDefaultTranslatorUIControll
+    $settings['weighting']['weighting_duration'] = array(
+      '#type' => 'select',
+      '#title' => t('Quick delivery'),
+      '#options' => drupal_map_assoc(array(20, 25, 30, 35, 40, 45, 50, 55, 60, 65, 70, 75, 80)),
+      // Default value taken from https://www.nativy.com/connectdoc/apimethods/postorder.aspx
+      '#default_value' => isset($default_weightings['weighting_duration']) ? $default_weightings['weighting_duration'] : 60,
+      '#description' => t('Indicates importance of the delivery time.'),
+    );

Not necessary to repeat that 4x IMHO (the defaut value comment.). And needs a .

+++ b/translators/nativy/tmgmt_nativy.ui.incundefined
@@ -14,18 +14,251 @@ class TMGMTNativyTranslatorUIController extends TMGMTDefaultTranslatorUIControll
+      '#description' => t('Indicates importance of the usage of a previous translation cooperation.'),
+    );
+    ¶
+    $settings['request_order'] = array(
+      '#type' => 'submit',

Trailing spaces.

berdir’s picture

Noticed two issues while demonstrating the current version:

- After fetching the job manually, a incorrect/strange status was displayed (don't remember it exactly, some kind of error)
- The callback from nativy.com somehow didn't worked, even though we were on a publicly available server.

corvus_ch’s picture

Status: Needs work » Needs review
StatusFileSize
new63.89 KB

Applyed review from comment #10

corvus_ch’s picture

Status: Fixed » Needs review
StatusFileSize
new63.92 KB

For the status thing I opened a new issue #1592138: Acces denied when checking translation status.

berdir’s picture

Status: Needs review » Fixed

Ok, commited, we'll improve it further in follow-ups.

PS: When referencing issues, make sure to wrap it in [ ] so that it is linked automatically.

Status: Needs review » Closed (fixed)

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