Currently, the Gengo plugin in the order summary lists the date and word count for the TMGMT record. However, the summary in the UI does not show the date they were ordered through Gengo and the Gengo word count. This makes billing clients and comparing their records very difficult and requires matching records on a database level.

Users of the plugin need a proper breakdown within the plugin to compare their orders to, because the mapping for the plugin is not straightforward.

Please let me know if this needs any further clarification.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Hi

Not sure we understand what you mean, as you are using different terms I think than we usually do for our lists and pages.

Is it possible to provide us with some screenshots and show on them where you expect what information? (Per e-mail if it's private data, or do it on a test site with fake data, shouldn't matter).

Berdir’s picture

One thing we've planned to do is shows information about the table where we store mapping information from our job to yours, including the involved amounts/credits. See #1891832: Add remote translator job mapping / history UI.

issam.zeibak’s picture

This feature looks like it will solve this issue. Let's talk about it during our next meeting.

blueminds’s picture

Here is the gengo implementation of #1891832. It will show the mapping history above messages as you can see in the attached screenshot.

Status: Needs review » Needs work

The last submitted patch, tmgmt_mygengo-remote_jobs_mappings-2047641-1.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
  1. +++ b/tmgmt_mygengo.ui.inc
    @@ -12,6 +12,43 @@
    +    $header = array(
    +      'Job item',
    +      'Data item',
    +      'Gengo word count',
    +      'Charged credits',
    +      'Gengo ID',
    +    );
    

    Missing t()

  2. +++ b/tmgmt_mygengo.ui.inc
    @@ -12,6 +12,43 @@
    +    /** @var TMGMTJobItem $item */
    +    foreach ($job->getItems() as $item) {
    

    We should document functions like this as @return TMGMTJobItem[], then this wouldn't be necessary.

- Some sort of default implementation would be nice, but is probably tricky. Gengo lists data items, others might just list job items, different labels/columns, .. so let's add it like this for now and then later on consolidate if there's potential for code re-use.

A summary row at the bottom would be useful, for the amount/word count.

issam.zeibak’s picture

I really like this solution as an intern, thank you @blueminds
I do second the summary for the amount/word count at the bottom row.

Berdir’s picture

Status: Needs review » Needs work
Berdir’s picture

Version: 7.x-1.0-alpha3 » 7.x-1.x-dev
blueminds’s picture

Status: Needs work » Needs review
FileSize
5.03 KB
89.2 KB

Changed based on the feedback - see the result in the screenshot.

Status: Needs review » Needs work

The last submitted patch, tmgmt_mygengo-remote_jobs_mappings-2047641-2.patch, failed testing.

blueminds’s picture

Status: Needs work » Needs review
FileSize
4.8 KB

fixing tests

Berdir’s picture

Status: Needs review » Needs work

Looks good.

However, as expected, there's not enough space. You picked a nice short example, but node titles are usually way longer.

Suggestion: Kill the tmgmt core CSS (not here, in core) to have those two below each other, not on the same level. I think there's an open issue where @cgalli tried to make it wider because we have problems with it in other translators too.

miro_dietiker’s picture

Please add a test that has multiple remote mappings and check the total.

Also, explicitly format the currency as floating calculations can lead to strange side effects.

blueminds’s picture

tests reworked to be more flexible and testing all mapping data.

The UI part is being solved here #2109757

miro_dietiker’s picture

Status: Needs review » Needs work

Test now cleanly tests multiple mappings with total checks.

I still don't see any explicit currency/credit formatting like previously requested:
"Also, explicitly format the currency as floating calculations can lead to strange side effects."

Berdir’s picture

  1. +++ b/tmgmt_mygengo.ui.inc
    @@ -495,6 +502,56 @@ class TMGMTMyGengoTranslatorUIController extends TMGMTDefaultTranslatorUIControl
    +          $credits,
    

    Yes, let's do a round($credits, 2) here and on the summary, to be sure that it's ok.

    Currency is a bit weird. They don't really show it either in their UI, but you can select $/EUR/Pound/Yen in their backend then the displayed amount is apparently re-calculated (no idea based on what), and we do display it on the job checkout.

    I think it's always the same for a given job (it is whatever you had configured at that time in the backend), so we don't need to repeat it. However, I do agree that it would be useful to show, maybe only in the total or in the table header (or both).

    Instead of duplicating it for every remote mapping, where we might not even have the information (not sure), what about storing it in the translator settings array of the job? We can add that here and check if it's there and if yes, display it, otherwise not. So old jobs won't have it, but new ones will.

  2. +++ b/tmgmt_mygengo.ui.inc
    @@ -495,6 +502,56 @@ class TMGMTMyGengoTranslatorUIController extends TMGMTDefaultTranslatorUIControl
    +          !empty($remote_mapping->remote_identifier_2) ? $remote_mapping->remote_identifier_2 : t('N/A'),
    

    What about linking this to the Gengo backend?

    The pattern is easy enough, it's e.g. http://sandbox.gengo.com/s_details/395256/. We only need to care about sandbox/real and then append the id. That seems quite useful as you can simply click on the number and do something in their backend.

    We can leave to just display the ID and add a "View in Gengo dashboard" or so as a link title.

Berdir’s picture

Also, I think just "Total" works better than "Total counts:" I actually doubt that's correct english, anyway...

Berdir’s picture

Also consider the following interdiff that we discussed. Making the remote info a by default collapsed fieldset (there could be hundreds of rows in there) and move the fetch translations button above it.

Additionally, we think it's inconsistent and not really useful to make the job item a link, if you have jobs with 20 fields, you will have 20 links to the top of the review page, so it might actually be more useful to have the data item clickable with a #target, but I suggest we don't do that yet.

blueminds’s picture

see the patch

Berdir’s picture

+++ b/tmgmt_mygengo.plugin.inc
@@ -448,8 +450,10 @@ class TMGMTMyGengoTranslatorPluginController extends TMGMTDefaultTranslatorPlugi
               'credits' => $response_job->credits,
+              'currency' => $response_job->currency,
               'tier' => $response_job->tier,

Yes, so that adds a lot of overhead as we save the information that's going to be the same for all remote mappings of a job over and over again.

And the display logic is a bit weird, as we just loop over the mappings and end up using whatever's written in the last one.

As suggested above, wondering if it would be easier to put that information into $job->settings, e.g. by adding a #type value to the checkout form, which should result in that being added there.

miro_dietiker’s picture

It is quite common that currency is repeated with transactional records. Not as a label, but as a currency code.

Usually, you want to be able to interprete persisted amounts without requesting any additional plugin. If you deduplicate the currency by moving it somewhere to the job (where only a specific translator plugin knows), the column of credits on its own is almost valueless.
Thus i would alter the schema and add the currency column. Even if you feel it's crazy redundancy.

Regarding looping and using the last one: Once our integrity enforcement issue is in, we have a guarantee about currency consistency inside a job.
#2106637: Change cancel to no longer allow re-submit, allow to create a new job with the same job items instead
It doesn't matter if you use the first, last, ... whatever for the total.

blueminds’s picture

Here's the thing... the currency is part of the remote_data field. That makes it practically same as being part of the job settings junk, just redundant. I suggest to introduce two extra columns: amount (decimal) and currency (char 3). This way it meets the conditions miro described.

blueminds’s picture

Here is the patch for having the amount and currency as a separate fields. To have this working this patch needs to be applied for core: https://drupal.org/node/2112077#comment-7966753

Status: Needs review » Needs work

The last submitted patch, tmgmt_mygengo-remote_jobs_mappings-2047641-6.patch, failed testing.

blueminds’s picture

Implementing amountToInt() and amountToFloat() methods.

To have this working this patch needs to be applied to core: https://drupal.org/node/2112077#comment-8007809

Status: Needs review » Needs work

The last submitted patch, tmgmt_mygengo-remote_jobs_mappings-2047641-7.patch, failed testing.

blueminds’s picture

see the patch

tests will fail - this needs to get into tmgmt core: https://drupal.org/comment/8171373#comment-8171373

Status: Needs review » Needs work

The last submitted patch, 28: tmgmt_mygengo-remote_jobs_mappings-2047641-8.patch, failed testing.

Berdir’s picture

Dependency has been committed, but we need a new release before we can commit this.