Refactor of Find a Job export code

The principle behind this refactor is to use Composition instead of Inheritance. Now the Upload class is not a God object but a class with a single responsibility: Upload the XML given contents into the DWP SFTP server Inbox. The orchrestration of the query-->xml-->upload is now managed by each service class: - PublishedAndUpdated - ClosedEarly The names of these classes have been adapted to match Teaching Vacancies domain language. These classes are not anaemic classes anymore.
DFE-Digital · Jun 21, 2024 · 37a55c7 · 37a55c7
1 parent bb40f5f
commit 37a55c7
Show file tree

Hide file tree

Showing 20 changed files with 194 additions and 140 deletions.
diff --git a/app/services/vacancies/export/dwp_find_a_job/closed_early.rb b/app/services/vacancies/export/dwp_find_a_job/closed_early.rb
@@ -0,0 +1,26 @@
+module Vacancies::Export::DwpFindAJob
+  class ClosedEarly
+    include Vacancies::Export::DwpFindAJob::ClosedEarlyVacancies
+
+    attr_reader :from_date
+
+    def initialize(from_date)
+      @from_date = from_date
+    end
+
+    def call
+      vacancies = Query.new(from_date).vacancies
+      xml = Xml.new(vacancies).xml
+
+      Upload.new(xml: xml, filename: filename).call
+
+      Rails.logger.info("[DWP Find a Job] Uploaded '#{filename}.xml': Containing #{vacancies.size} vacancies to close.")
+    end
+
+    private
+
+    def filename
+      @filename ||= "TeachingVacancies-expire-#{Time.zone.now.strftime('%Y%m%d-%H%M%S')}"
+    end
+  end
+end
diff --git a/...p_find_a_job/expired_and_deleted/query.rb → ...ind_a_job/closed_early_vacancies/query.rb b/...p_find_a_job/expired_and_deleted/query.rb → ...ind_a_job/closed_early_vacancies/query.rb
@@ -1,4 +1,4 @@
-module Vacancies::Export::DwpFindAJob::ExpiredAndDeleted
+module Vacancies::Export::DwpFindAJob::ClosedEarlyVacancies
   class Query
     attr_reader :from_date
 

diff --git a/...dwp_find_a_job/expired_and_deleted/xml.rb → ..._find_a_job/closed_early_vacancies/xml.rb b/...dwp_find_a_job/expired_and_deleted/xml.rb → ..._find_a_job/closed_early_vacancies/xml.rb
@@ -1,6 +1,6 @@
 require "nokogiri"
 
-module Vacancies::Export::DwpFindAJob::ExpiredAndDeleted
+module Vacancies::Export::DwpFindAJob::ClosedEarlyVacancies
   class Xml
     attr_reader :vacancies
 

diff --git a/app/services/vacancies/export/dwp_find_a_job/expired_and_deleted/upload.rb b/app/services/vacancies/export/dwp_find_a_job/expired_and_deleted/upload.rb
diff --git a/app/services/vacancies/export/dwp_find_a_job/new_and_edited/upload.rb b/app/services/vacancies/export/dwp_find_a_job/new_and_edited/upload.rb
diff --git a/app/services/vacancies/export/dwp_find_a_job/published_and_updated.rb b/app/services/vacancies/export/dwp_find_a_job/published_and_updated.rb
@@ -0,0 +1,26 @@
+module Vacancies::Export::DwpFindAJob
+  class PublishedAndUpdated
+    include Vacancies::Export::DwpFindAJob::PublishedAndUpdatedVacancies
+
+    attr_reader :from_date
+
+    def initialize(from_date)
+      @from_date = from_date
+    end
+
+    def call
+      vacancies = Query.new(from_date).vacancies
+      xml = Xml.new(vacancies).xml
+
+      Upload.new(xml: xml, filename: filename).call
+
+      Rails.logger.info("[DWP Find a Job] Uploaded '#{filename}.xml': Containing #{vacancies.size} vacancies to publish.")
+    end
+
+    private
+
+    def filename
+      @filename ||= "TeachingVacancies-upload-#{Time.zone.now.strftime('%Y%m%d-%H%M%S')}"
+    end
+  end
+end
diff --git a/...nd_a_job/new_and_edited/parsed_vacancy.rb → ...d_and_updated_vacancies/parsed_vacancy.rb b/...nd_a_job/new_and_edited/parsed_vacancy.rb → ...d_and_updated_vacancies/parsed_vacancy.rb
@@ -1,6 +1,6 @@
 require "action_text"
 
-module Vacancies::Export::DwpFindAJob::NewAndEdited
+module Vacancies::Export::DwpFindAJob::PublishedAndUpdatedVacancies
   class ParsedVacancy
     include ActionView::Helpers::SanitizeHelper
     include Rails.application.routes.url_helpers

diff --git a/...rt/dwp_find_a_job/new_and_edited/query.rb → .../published_and_updated_vacancies/query.rb b/...rt/dwp_find_a_job/new_and_edited/query.rb → .../published_and_updated_vacancies/query.rb
@@ -1,4 +1,4 @@
-module Vacancies::Export::DwpFindAJob::NewAndEdited
+module Vacancies::Export::DwpFindAJob::PublishedAndUpdatedVacancies
   class Query
     # Find a Job only accepts "expiry" dates up to 30 days from the date of export/update.
     FIND_A_JOB_MAX_EXPIRY_DAYS = 30

diff --git a/...port/dwp_find_a_job/new_and_edited/xml.rb → ...ob/published_and_updated_vacancies/xml.rb b/...port/dwp_find_a_job/new_and_edited/xml.rb → ...ob/published_and_updated_vacancies/xml.rb
@@ -1,6 +1,6 @@
 require "nokogiri"
 
-module Vacancies::Export::DwpFindAJob::NewAndEdited
+module Vacancies::Export::DwpFindAJob::PublishedAndUpdatedVacancies
   class Xml
     APPLY_VIA_EXTERNAL_URL_ID = 2
 

diff --git a/app/services/vacancies/export/dwp_find_a_job/upload.rb b/app/services/vacancies/export/dwp_find_a_job/upload.rb
@@ -0,0 +1,32 @@
+module Vacancies::Export::DwpFindAJob
+  class Upload
+    attr_reader :xml, :filename
+
+    def initialize(xml:, filename:)
+      @xml = xml
+      @filename = filename
+    end
+
+    def call
+      file = Tempfile.new(filename)
+      begin
+        file.write(xml)
+        file.flush # Ensure all data is written to disk before uploading
+        upload_to_find_a_job_sftp(file.path)
+      ensure
+        file.close!
+      end
+    end
+
+    private
+
+    def upload_to_find_a_job_sftp(file_path)
+      Net::SFTP.start(ENV.fetch("FIND_A_JOB_FTP_HOST", ""),
+                      ENV.fetch("FIND_A_JOB_FTP_USER", ""),
+                      password: ENV.fetch("FIND_A_JOB_FTP_PASSWORD", ""),
+                      port: ENV.fetch("FIND_A_JOB_FTP_PORT", "")) do |sftp|
+        sftp.upload!(file_path, "Inbound/#{filename}.xml")
+      end
+    end
+  end
+end
diff --git a/app/services/vacancies/export/dwp_find_a_job/upload_base.rb b/app/services/vacancies/export/dwp_find_a_job/upload_base.rb
diff --git a/documentation/integrations/dwp-find-a-job.md b/documentation/integrations/dwp-find-a-job.md
@@ -7,8 +7,8 @@ Find a job service integration is done through daily XML bulk uploads to their S
 
 There are 2 bulk upload types:
 
-1. Uploading new/edited vacancies.
-2. Uploading expired/deleted vacancies.
+1. Uploading new/edited vacancies (published/updated on TV vacancies)
+2. Uploading expired/deleted vacancies (manually closed early on TV vacancies)
 
 The bulk upload file/s are imported once per day by Find a job service.
 
@@ -22,19 +22,19 @@ block:TeachingVacancies:3
     TeachingVacanciesTag>"Teaching Vacancies"] space:2
     space Database[("Database")] space
 
-    NewAndEdited["NewAndEdited"]
+    PublishedAndUpdated["PublishedAndUpdated"]
     space
-    ExpiredAndDeleted["ExpiredAndDeleted"]
+    ClosedEarly["ClosedEarly"]
 
-    Database --> NewAndEdited
-    Database --> ExpiredAndDeleted
+    Database --> PublishedAndUpdated
+    Database --> ClosedEarly
 
     XmlPublish[\"XML: to publish/update"/]
     space
     XmlDelete[\"XML: to delete"/]
 
-    NewAndEdited-- "Generates" -->XmlPublish
-    ExpiredAndDeleted-- "Generates" -->XmlDelete
+    PublishedAndUpdated-- "Generates" -->XmlPublish
+    ClosedEarly-- "Generates" -->XmlDelete
 end
 style TeachingVacancies fill:#e6f2ff,stroke:#333,stroke-width:2px
 
@@ -80,43 +80,39 @@ The process has 3 stages:
 2. Generate a bulk XML temporal file from those vacancies following Find a Job file specs.
 3. Push the file to the Find a Job service SFTP server.
 
-This stages are orchrestated by an [abstract upload class](../../app/services/vacancies/export/dwp_find_a_job/upload_base.rb)
-
-The classes in charge of generating and pushing the new/edited and deleted files use this base class and only need to define:
-- what query is done
-- how the XML file is generated (the parsing from Vacancy to XML)
-- the name of the file to upload to Find a Job service SFTP server.
-
 ```mermaid
 ---
 title: DWP Find a Job integration code structure
 ---
 classDiagram
-   note for UploadBase "NOTE: Implements the behaviour but child classes need to define:\nFILENAME_PREFIX\nQUERY_CLASS\nXML_CLASS"
-  UploadBase <|-- NewUpload : implements
-  UploadBase <|-- ExpiredUpload : implements
+  PublishedAndUpdated *-- Upload : composition
+  ClosedEarly *-- Upload : composition
   NewXml *-- ParsedVacancy : composition
-  NewUpload *-- NewXml : composition
-  NewUpload *-- NewQuery : composition
-  ExpiredUpload *-- ExpiredXml : composition
-  ExpiredUpload *-- ExpiredQuery : composition
+  PublishedAndUpdated *-- NewXml : composition
+  PublishedAndUpdated *-- NewQuery : composition
+  ClosedEarly *-- ExpiredXml : composition
+  ClosedEarly *-- ExpiredQuery : composition
 
-  class UploadBase {
-    <<Abstract>>
+  class PublishedAndUpdated {
     from_date
     +call()
-    -upload_to_find_a_job_sftp(file_path)
     -filename()
-    -log_upload(vacancies_number)
   }
 
-  namespace NewAndEdited {
-    class NewUpload["Upload"] {
-      FILENAME_PREFIX
-      QUERY_CLASS
-      XML_CLASS
-    }
+  class ClosedEarly {
+    from_date
+    +call()
+    -filename()
+  }
 
+  class Upload {
+    xml
+    filename
+    +call()
+    -upload_to_find_a_job_sftp(file_path)
+  }
+
+  namespace PublishedAndUpdatedVacancies {
     class NewXml["Xml"] {
       vacancies
       +xml()
@@ -149,13 +145,7 @@ classDiagram
 
   }
 
-  namespace ExpiredAndDeleted {
-    class ExpiredUpload["Upload"] {
-      FILENAME_PREFIX
-      QUERY_CLASS
-      XML_CLASS
-    }
-
+  namespace ClosedEarlyVacancies {
     class ExpiredXml["Xml"] {
       vacancies
       +xml()
@@ -168,9 +158,9 @@ classDiagram
   }
 ```
 
-## Uploading new/edited vacancies
+## Uploading published/updated vacancies
 
-The service orcherstating the upload of new/edited vacancies is [Vacancies::Export::DwpFindAJob::NewAndEdited::Upload](../../app/services/vacancies/export/dwp_find_a_job/new_and_edited/upload.rb)
+The service orcherstating the upload of new/edited vacancies is [Vacancies::Export::DwpFindAJob::PublishedAndUpdated](../../app/services/vacancies/export/dwp_find_a_job/published_and_updated.rb)
 
 ### What vacancies are exported?
 We publish vacancies that fall on any of these conditions:
@@ -209,7 +199,6 @@ classDef querybit fill:#ffffcc,stroke:#333;
 classDef datasource fill:#ffcc99,stroke:#333;
 class RecentlyPublished,RecentlyUpdated,NeedToSetExpiryDate,NeedToPushExpiryDate querybit
 class Database datasource
-
 ```
 
 ### Vacancies that need their expiry date updated/pushed back
@@ -246,9 +235,9 @@ This would:
 - Pro: Reduce DB query complexity. As we would only need to query: "Any vacancies with expiry date over 30 days from today".
 - Con: Increase the size of the XML files we push into DWP Find a Job service, as now selects "all the vacancies over 30 days..." instead of a subset of those.
 
-## Uploading expired/deleted vacancies
+## Uploading vacancies closed early
 
-The service orcherstating the upload of new/edited vacancies is [Vacancies::Export::DwpFindAJob::ExpiredAndDeleted::Upload](../../app/services/vacancies/export/dwp_find_a_job/expired_and_deleted/upload.rb)
+The service orcherstating the upload of vacancies closed early is [Vacancies::Export::DwpFindAJob::ClosedEarly](../../app/services/vacancies/export/dwp_find_a_job/closed_early.rb)
 
 ### What vacancies are exported?