Etda Committee import integration by usmannsiddiqui · Pull Request #152 · psu-libraries/fams_tools

usmannsiddiqui · 2026-03-19T14:35:23Z

Closes #143
Closes #145
Close #132 - as a whole

Brief Technical Summary:

Adds client that pulls committee data from ETDA
Adds a job that handles the full integration from ETDA to fams_tools
Maps the committee roles to activity insight
Created the envrc.example in FAMS Tools for secrets template

- Fill in all test cases using WebMock to stub HTTP responses - Fix RSpec.describe class name and capitalization - Fix auth header from Bearer token to X-API-KEY

Adds CommitteeData::EtdaImporter with import_all and import_for_faculty methods. Iterates over all faculty, calls the ETDA API client, and creates committee records. Error handling and tests still in progress.

- Add CommitteeData::EtdaImporter.new.import_all call to integrate method - Pass faculty object directly to import_for_faculty instead of access_id - Move import_for_faculty to private and fix access_id references

…nstead of a hash - Rename job to 'Committee Integration' - Use send(:name) in spec to test private method

… handling

- Fix stage_of_completion: 'In Progress' -> 'In Process' to match Activity Insight's valid dropdown values; add 'Withdrew' support - Add role_other_explanation and type_other_explanation DB columns for when role/type maps to 'Other' in Activity Insight - Improve map_type_of_work to use role context and handle more degree types (undergraduate, postdoc, honors) - Emit <ROLE_OTHER> and <TYPE_OTHER> XML tags when explanations present - Update all related specs

…eline

…mittees

Alex Kiessling (ajkiessl)

I have a handful of small change requests. Otherwise, this looks very close to being done!

Alex Kiessling (ajkiessl) · 2026-03-31T19:13:01Z

              xml.DSL do
                xml.ROLE committee.role
+                xml.TYPE committee.type_of_work
+                xml.TYPE_OTHER committee.type_other_explanation if committee.type_other_explanation.present?


Can this be removed?

Hey Alex, removed.

Alex Kiessling (ajkiessl) · 2026-03-31T19:15:32Z

+class AddOtherExplanationFieldsToCommittees < ActiveRecord::Migration[7.2]
+  def change
+    change_table :committees, bulk: true do |t|
+      t.text :role_other_explanation, limit: 20_000
+      t.text :type_other_explanation, limit: 20_000
+    end
+  end
+end


Let's just delete this migration file. Since it hasn't been deployed, we don't need to keep this at all.

Alex Kiessling (ajkiessl) · 2026-03-31T19:17:23Z

+  def change
+    change_table :committees, bulk: true do |t|
+      t.remove :degree_type, type: :string
+      t.remove :role_other_explanation, type: :text


Remove line 5 since we will completely delete the migration that adds this field. Also, is t.remove the idiomatic way to do this? Just curious where you got that from since I think I usually use remove_column.

you're right, its good for big changes but in this case I was removing one column so I will change it back to remove_column.

Alex Kiessling (ajkiessl) · 2026-03-31T19:18:33Z

+    t.string "type_of_work"
+    t.string "stage_of_completion"
+    t.integer "start_year"
+    t.integer "completion_year"
+    t.text "type_other_explanation"


type_other_explanation needs removed. It should get removed when you run the migrations after the fixes I suggested.

Alex Kiessling (ajkiessl) · 2026-03-31T19:22:28Z

    it { is_expected.to have_db_column(:student_lname).of_type(:string) }
    it { is_expected.to have_db_column(:role).of_type(:string) }
    it { is_expected.to have_db_column(:thesis_title).of_type(:string) }
-    it { is_expected.to have_db_column(:degree_type).of_type(:string) }
    it { is_expected.to have_db_column(:faculty_id).of_type(:integer) }


We should add tests for the new db_columns we added.

Added column tests for type_of_work, stage_of_completion, start_year, completion_year

Alex Kiessling (ajkiessl) · 2026-03-31T19:23:04Z

+importer = CommitteeData::EtdaImporter.new
+Committee.last.attributes
+ENV['ETDA_API_URL']
+quit
+importer = CommitteeData::EtdaImporter.new
+Committee.last.attributes
+importer.import_all
+quit
+importer = CommitteeData::EtdaImporter.new
+importer.import_all
+quit


This can be removed. I think the .gitignore got messed up. This file should be ignored.

added to gitignore

Since this file is already committed, you'll have to remove it from git manually. Something like git rm path/to/file

Alex Kiessling (ajkiessl) · 2026-03-31T19:23:32Z

  Add the following untracked files/folders:

-  * `config/database.yml` This will be similar to `config/database.yml.sample`  
+  * `'config/database.yml'` This will be similar to `config/database.yml.sample`  


Not sure how this change got in here, but it can be reverted.

deleted this.

…date specs and gitignore

usmannsiddiqui · 2026-03-31T20:16:32Z

I have made all the changes that were requested, hopefully this should be good now!

Alex Kiessling (ajkiessl)

A couple more things

Alex Kiessling (ajkiessl) · 2026-04-01T16:53:34Z

+importer = CommitteeData::EtdaImporter.new
+Committee.last.attributes
+ENV['ETDA_API_URL']
+quit
+importer = CommitteeData::EtdaImporter.new
+Committee.last.attributes
+importer.import_all
+quit
+importer = CommitteeData::EtdaImporter.new
+importer.import_all
+quit


Since this file is already committed, you'll have to remove it from git manually. Something like git rm path/to/file

Alex Kiessling (ajkiessl) · 2026-04-01T16:54:33Z


  Add the following untracked files/folders:
-
-  * `config/database.yml` This will be similar to `config/database.yml.sample`  


This line should be added back, I just wanted you to remove the extra string quotes that were added.

Added back the config/database.yml line

About .irb_history file. Removed it from git tracking with git rm --cached .irb_history

Alex Kiessling (ajkiessl) · 2026-04-01T16:55:10Z

You'll need to merge in main and resolve the conflict with the Dockerfile.

Alex Kiessling (ajkiessl) · 2026-04-01T17:27:34Z

Also, we need some logic in here to prevent imports of committees from before "some date". We don't want to import everything. Did we want to set the limit to committees that have started the approval process in the last 6 months?

…racking, fix README

usmannsiddiqui · 2026-04-01T22:21:31Z

You're right about the setting the limit, I remember Nicole and Binky saying that we want to import people who started their approval process within a 6 month range.

Added a new filter that rejects anything older than 6 months.

etda_importer.rb: New private method within_last_six_months?, this will take a date string, returns false if blank, otherwise checks if the date is within the last 6 months using 6.months.ago.

etda_importer_spec.rb: updated specs accordingly.

…rror in spec - Changed logic for start_year in committee_xml_builder

Alex Kiessling (ajkiessl)

LGTM

Joao Vitor Barros da Silva (jvitorbarros15) and others added 28 commits February 4, 2026 10:52

Add basic regex to map roles from ETDA API

c723598

Fix committee_role_normalizer to pass Rspec

9ab610c

Fix Dockerfile with updated image version and rubocop corrections

4d4a22c

Rubocop

f2fbf9d

Add edta api importer

8eb2396

Fix name to etda_committee_memberships_xml_builder.rb

9ae71a2

Add rspec tests for the api importer

804bc0f

Refactor ETDA committee memberships XML builder to use Nokogiri

f887c48

Setup skeleton for the api client with initialize

e898cc7

Skeleton initialized, didnt save file initially

15f502a

Finished setting up the parameters for faculty committee

f488a2b

Created rspec tests for committee records client

eb0e15c

Add .envrc.example for environment configuration

21063fa

Add environment variables to .envrc.example

e8df657

Fix API client to use env variable methods

2bbed27

changed explanation in envr.example

f76696c

Complete RSpec tests for CommitteeRecordsClient and fix API key header

a40040c

- Fill in all test cases using WebMock to stub HTTP responses - Fix RSpec.describe class name and capitalization - Fix auth header from Bearer token to X-API-KEY

Add Committee.delete_all to ApplicationJob cleanup

740d5bc

Finished rspec tests for committee_records_client

70bff3c

WIP: Add EtdaImporter skeleton for pulling committee data from ETDA API

54279de

Adds CommitteeData::EtdaImporter with import_all and import_for_faculty methods. Iterates over all faculty, calls the ETDA API client, and creates committee records. Error handling and tests still in progress.

Merge branch 'api-client-etda' into etda-post-receiver

8b3fd0a

Wire EtdaImporter into ActivityInsightCommitteeJob and fix importer

be3fc7a

- Add CommitteeData::EtdaImporter.new.import_all call to integrate method - Pass faculty object directly to import_for_faculty instead of access_id - Move import_for_faculty to private and fix access_id references

Modified the etda importer and created rspec tests for the etda importer

8fe4f47

adding rubocop fixes for now

ebc0229

Rspec tests passed - Fix integrate method to accept target directly i…

3a0ef49

…nstead of a hash - Rename job to 'Committee Integration' - Use send(:name) in spec to test private method

Fix etda importer spec to correctly test committee creation and error…

3011030

… handling

Fix rspec tests for committee records client

aa0093a

deleted 2 redundant files

a832109

usmannsiddiqui requested review from Alex Kiessling (ajkiessl) and Jesse LE (jlandiseigsti) March 19, 2026 14:41

usmannsiddiqui requested review from Joao Vitor Barros da Silva (jvitorbarros15) and Madhura Khandkar (madhurakhandkar) March 19, 2026 14:41

usmannsiddiqui and others added 7 commits March 19, 2026 11:52

fixed up regex in committe normalizer and updated envrc.example

74c5441

Rubocop corrections

4c30e5a

Fix XML tag COMP -> COMPSTAGE to match Activity Insight schema

092e2f4

Remove wihtdrawn from determine_comp method

7cfbafb

Remove degree_type and role_other_explanation from committee data pip…

1f78a66

…eline

Add migration to drop degree_type and role_other_explanation from com…

1442e91

…mittees

Alex Kiessling (ajkiessl) requested changes Mar 31, 2026

View reviewed changes

Jesse LE (jlandiseigsti) mentioned this pull request Mar 31, 2026

Fix ldap check #158

Closed

Address PR feedback: remove type_other_explanation, fix migration, up…

1691ad8

…date specs and gitignore

Alex Kiessling (ajkiessl) requested changes Apr 1, 2026

View reviewed changes

Alex Kiessling (ajkiessl) marked this pull request as ready for review April 1, 2026 16:56

Filter committee imports to last 6 months, remove .irb_history from t…

2e902d9

…racking, fix README

usmannsiddiqui added 4 commits April 2, 2026 09:59

Update base Docker image to ruby-3.4.9-node-22

0eb5b3b

Merge remote-tracking branch 'origin/main' into etda-post-receiver

5366429

Removed type_other_explantion from schema

4dca339

Removed submission status from etda_importer and fixed rails logger e…

1e7b753

…rror in spec - Changed logic for start_year in committee_xml_builder

Alex Kiessling (ajkiessl) approved these changes Apr 9, 2026

View reviewed changes

Joao Vitor Barros da Silva (jvitorbarros15) merged commit 1476e5e into main Apr 9, 2026
4 checks passed


		Add the following untracked files/folders:

		* `config/database.yml` This will be similar to `config/database.yml.sample`

Conversation

usmannsiddiqui commented Mar 19, 2026

Uh oh!

Alex Kiessling (ajkiessl) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

usmannsiddiqui commented Mar 31, 2026

Uh oh!

Alex Kiessling (ajkiessl) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alex Kiessling (ajkiessl) commented Apr 1, 2026

Uh oh!

usmannsiddiqui commented Apr 1, 2026

Uh oh!

Alex Kiessling (ajkiessl) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants