Connector class

Looks good!

+++ b/tmgmt_mygengo.connector.incundefined
@@ -0,0 +1,269 @@
+    if (variable_get('tmgmt_mygengo_use_mock_service', FALSE)) {
+      $this->baseUrl = $GLOBALS['base_url'] . '/tmgmt_mygengo_mock';

What's the property for if it's hardcoded here?

+++ b/tmgmt_mygengo.connector.incundefined
@@ -0,0 +1,269 @@
+    return $this->request('translate/service/languages', 'GET');

Do we want to default to GET? Or maybe have get()/post() methods?

+++ b/tmgmt_mygengo.connector.incundefined
@@ -0,0 +1,269 @@
+   * @return object
+   *   Gengo response data.
+   */
+  function getJobs(array $gengo_job_ids) {
+    return $this->request('translate/jobs/' . implode(',', $gengo_job_ids), 'GET');

Given that this supports multiple jobs, does it really return an object?

+++ b/tmgmt_mygengo.connector.incundefined
@@ -0,0 +1,269 @@
+   * @param $method
+   *   HTTP method (GET, POST...)

@param string $method, when we're fixing all of those anyway...

+++ b/tmgmt_mygengo.connector.incundefined
@@ -0,0 +1,269 @@
+      watchdog('tmgmt_mygengo', "Sending request to gengo at @url with data @data\n\nResponse: @response", array(

Does it make sense to add the $method to the watchdog log? e.g., sending GET/POST request to gengo at ...

Awesome issue in terms of improving code quality and readability. Thanks for working on this! :)

This comment may come a bit late given that there's already a full patch, but have you considered using the official Gengo PHP library instead of a custom connector class? I could be wrong, but from a quick glance it looks like they do pretty much the same thing.

We discussed that for OHT already, the problem are licensing questions.

No idea what kind of license that is but it's not GPL and external libraries aren't allowed on d.o, even if it were. So we would have to require our users to manually download that and put it in a libraries folder.

Ok, had a look at the new plugin.inc file in the editor, looks a lot better already.

- There are two methods remaining that start with gengo, can we rename gengoFetchCommits to just fetchCommits()? It's the gengo translator plugin, it's quite obvious that it's not going to fetch OHT comments ;)

- Same for gengoSendJob(), I think just sendJob() is fine. What we should do instead is mark methods that are not supposed to be called from outside with protected. There are of course some methods that are, e.g. from the page callback. We could also refactor it to not actually send a request but only transform the $job to the data that we want to submit and then submit from within requestTranslation()/getQuote(). The only tricky part is the duplicated check handling, that would unfortunately still require the $quote argument. Still waiting on feedback from them if we can safely rely on their duplicated items response.

- Some methods are missing a docblock, .e.g getDefaultRemoteLanguagesMappings(). That should get the standard Implements ... header so that we know that this is an implementation of the interface and not a custom method.

- getGengoLanguagePairs() has support for a $target_language filter but that does not seem to be used anywhere, possibly a left-over of my refactoring for the supported remote languages? We might even just inline it into getSupportedTargetLanguages() because I don't think we actually need the static caching logic there, that's handled already in the translator entity for us.

- The mapping related methods can be cleaned up when converting that to use the core mapping API.

+++ b/CHANGELOG.txtundefined
@@ -4,6 +4,8 @@ TMGMT Translator Gengo 7.x-1.0-beta2, xxxx-xx-xx
 - Issue #1870770 - Remaining credits info on job checkout page
 - Issue #1922100 - Deal with duplicate content translations
 - Issue #1963936 - Job comments - basic commenting functionality
+- Issue #1968510 - Provide a connector class, major refactor of translator
+  plugin class and UI controller class

Should be at the top, newest entries in a change log are always at the top.

+++ b/tmgmt_mygengo.testundefined
@@ -76,7 +63,6 @@ class TMGMTMyGengoTestCase extends TMGMTBaseTestCase {
     $this->assertFalse(isset($languages['it']));
-    $this->assertFalse(isset($languages['en']));

Why is this removed? :)

Looks great, not sure about moving the functions to the UI controller but the actual API is the connector class now I guess, so if you want to re-use it somehow, just call that...

Great, commited and pushed.