vi-1055 Create UserActionEventsController and query route

[emilykim13]

Note: Delete the description statements, complete each step. None are optional, but can be justified as to why they cannot be completed as written. Provide known gaps to testing that may raise the risk of merging to production.

Summary

• No feature toggle
• This adds 2 new files, UserActionEventsController.rb and it's respective spec file. Changes include a new route to the #index action in this controller.
• Identity Platform Team

Related issue(s)

• Vi-1055

Testing done

• New code is covered by unit tests
• Confirmed routes have been created

bin/rails routes | grep user_action_events
                               v0_user_action_events GET                       /v0/user_action_events(.:format)                                                                  v0/user_action_events#index {:format=>"json"}
                  v0_user_action_events GET    /v0/user_action_events(.:format)                                             mobile/v0/user_action_events#index

What areas of the site does it impact?

• This a part of the Audit DB project

Acceptance criteria

• I fixed|updated|added unit tests and integration tests for each feature (if applicable).
• No error nor warning in the console.
• Events are being sent to the appropriate logging solution
• Documentation has been updated (link to documentation)
• No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
• Feature/bug has a monitor built into Datadog (if applicable)
• If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
• I added a screenshot of the developed feature

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[github-actions]

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: app/controllers/v0/user_action_events_controller.rb

[bramleyjl]

Thanks for making those earlier changes! The feature code itself looks pretty good, however in order for a PR to be ready for review the features/changes it introduces should be fully covered by unit specs & have complete manual testing instructions for calling the controller & the different parameters it can take.

Below are some notes on what should be in those testing instructions; my method is to record as much from the Rails console as I can when I'm developing and personally testing the feature, that way I have plenty to Copy + Paste from to make a comprehensive testing guide for the PR.

Since we're still currently developing the UserActionEvent seeding & UserAction instantiation you'd want to start with instructions for populating your DB with a UserActionEvent & using it to create one or more UserActions for testing.

user_verification = UserVerification.find_by(idme_uuid:<idme_uuid>)
user_action_event = UserActionEvent.new(details: 'test user action event').save!
user_action_one = UserAction.new(user_action_event_id: user_action_event.id, status: 'success', subject_user_verification_id: user_verification.id, acting_user_verification_id: user_verification.id, acting_ip_address: '::1', acting_user_agent: 'test')
# create more for specific test cases
# user_action_two = UserAction.new(user_action_event_id:..., created_at: 3.years.ago)
...
user_action_one.save!

Then add a cURL or just a URL with instructions on how to call the controller method and what you should be expecting for a response.

* Call the controller endpoint, authenticated through the CSP connected to the user_verification used to create the `UserAction` records.
    * `http://localhost:3000/v0/user_action_events`
* You should receive a 200 response with a JSON body containing the queried `UserAction` records under the `data` header and any implemented `UserActionEvent` records under the `included` header.
      {
          "data": [
              {
                  "id": "301b8f06-79f8-4bc4-a5e1-35930f7ee84d",
                  "type": "user_action",
                  "attributes": {
                      "user_action_event_id": 1,
                      "status": "success",
                      "subject_user_verification_id": 1,
                      "acting_ip_address": "::1",
                      "acting_user_agent": "test",
                      "created_at": "2025-02-12T18:09:14.137Z",
                      "updated_at": "2025-02-12T18:09:14.137Z",
                      "acting_user_verification_id": 1
                  },
                  "relationships": {
                      "user_action_event": {
                          "data": {
                              "id": "1",
                              "type": "user_action_event"
                          }
                      }
                  }
              }
          ],
          "included": [
              {
                  "id": "1",
                  "type": "user_action_event",
                  "attributes": {
                      "details": "test user action event",
                      "created_at": "2025-02-12T18:09:06.507Z",
                      "updated_at": "2025-02-12T18:09:06.507Z"
                  },
                  "relationships": {
                      "user_actions": {
                          "data": [
                              {
                                  "id": "301b8f06-79f8-4bc4-a5e1-35930f7ee84d",
                                  "type": "user_action"
                              }
                          ]
                      }
                  }
              }
          ]
      }

After the base test case you can specify the different parameter options that should be tested and what to expect when doing so.

[bramleyjl]

This is a good start, I'd update the spec to include demonstrations of filtering on both the start_date and end_date. A spec or two that cover the pagination behavior would also be a good idea.

[emilykim13]

Thank you @bramleyjl for the in depth review! In the new commits, I tried to be more intentional with the unit testing, here's a brief overview of those changes:
More intentional unit tests to better demonstrate the index action:

• Added 3 more contexts under the #index action describe block
  • context 'when there are no user actions'
    • it 'returns an empty array'
  • context 'user actions'
    • it 'returns user actions within date range' (this is to test without a pagination param)
    • it 'returns user actions by newest to oldest within date range' (checking the :desc order of user actions to sort for most recent user actions first)
    • Added context 'pagination' to test the pagination aspect of user actions in this query
      • it 'paginates the correct number of user actions per page'
      • it 'paginates user actions in order'
  • context 'user action events'
    • it 'returns a successful response'
    • it 'includes the user action event'

I left some comments to help clarify the intention behind the test, lmk if I should remove those. Ty again!

[bramleyjl]

I forgot to mention you'll also need to update the .github/CODEOWNERS file in order to pass the Codeowners check - the Identity team and others tag files that they create so that their review is required when anyone attempts to change that file. So these files:

• app/controllers/v0/user_action_events_controller.rb
• app/serializers/user_action_event_serializer.rb
• app/serializers/user_action_serializer.rb
• spec/controllers/v0/user_action_events_controller_spec.rb

all need to added with our team name like so:

app/controllers/v0/user_action_events_controller.rb @department-of-veterans-affairs/octo-identity

There have been some efforts at organizing and alphabetizing CODEOWNERS so please put each new line among the others in its general area: app/controllers/v0, app/serializers, and spec/controllers/v0.

[bramleyjl]

Good progress! I left some more comments about ways that the spec file can be improved upon to fully cover the different aspects of the search behavior.

There still needs to be a full manual testing guide in the PR writeup, see this comment for some instructions on what that should look like. It should be complete enough that your tester should be able to mostly copy + paste code examples to reproduce the behavior you want them to see. It will also uncover areas of the PR that still need work; for example I discovered an inability to determine what page I was on in the response body when attempted to test the pagination and create a unit spec for it. Let me know if you'd like to pair on the testing & PR writeup together.

[bramleyjl]

In general we try to comment as little as possible; for unit specs the it... do and context blocks should be explanatory enough. We can remove this first test as its functionality is replicated by the start_date and end_date filtering tests below.

[bramleyjl]

This test is several separate things, the start_date filtering, end_date filtering, the sorting of the response, and the attributes of the response object. It's easiest for other engineers to quickly grok what your tests are doing if you keep them as small & specific as possible. So I'd take the "returns user actions within date range" context and apply that to a start and end date filtering test.
I'm also realizing that our controller method has defaults that it uses when no parameter is present; that behavior should be covered by a test as well, so this will require some nested context blocks.

  context 'when filtering by date range' do
    let(:start_date) { 3.months.ago.to_date }
    let(:end_date) { 2.weeks.ago.to_date }

    context 'when the start date and/or end dates are provided' do
      it 'returns user actions within the date range' do
        get :index, params: { start_date:, end_date: }

        json_response = JSON.parse(response.body)['data']
        expect(json_response.length).to eq(2)
        expect(json_response.first['id']).to eq(user_action_three.id)
        expect(Time.zone.parse(json_response.first['attributes']['created_at'])).to be <= end_date
        expect(json_response.second['id']).to eq(user_action_two.id)
        expect(Time.zone.parse(json_response.second['attributes']['created_at'])).to be >= start_date
      end
    end

    context 'when the start date is not provided' do
      let(:start_date) { nil }
      let(:expected_start_date) { 1.month.ago.to_date }

      it 'returns user actions within the past month' do
        get :index, params: { start_date:, end_date: }

        json_response = JSON.parse(response.body)['data']
        expect(json_response.length).to eq(1)
        expect(json_response.first['id']).to eq(user_action_three.id)
        expect(Time.zone.parse(json_response.first['attributes']['created_at'])).to be >= expected_start_date
      end
    end

    context 'when the end date is not provided' do
      let(:end_date) { nil }
      let(:expected_end_date) { Time.zone.now }

      it 'returns user actions up to the current date' do
        get :index, params: { start_date:, end_date: }

        json_response = JSON.parse(response.body)['data']
        expect(json_response.length).to eq(3)
        expect(json_response.first['id']).to eq(user_action_four.id)
        expect(Time.zone.parse(json_response.first['attributes']['created_at'])).to be <= expected_end_date
        expect(json_response.second['id']).to eq(user_action_three.id)
        expect(json_response.third['id']).to eq(user_action_two.id)
      end
    end
  end

Beneath the filtering block you can do individual tests for the sorting.

  it 'returns the results in descending order by created_at' do
    get :index, params: { start_date: 3.months.ago.to_date }

    json_response = JSON.parse(response.body)['data']
    expect(json_response.length).to eq(3)

    first_created_at = json_response.first['attributes']['created_at']
    second_created_at = json_response.second['attributes']['created_at']
    third_created_at = json_response.third['attributes']['created_at']
    expect(first_created_at).to be > second_created_at
    expect(second_created_at).to be > third_created_at
  end

And then the attribute checks, which I'd do after the pagination block to keep all of the filtering, sorting, and presentation tests together.

  context 'when returning user actions' do
    let(:expected_attributes) do
      {
        'user_action_event_id' => user_action_event_two.id,
        'status' => user_action_four.status,
        'subject_user_verification_id' => user_verification.id,
        'acting_ip_address' => user_action_four.acting_ip_address,
        'acting_user_agent' => user_action_four.acting_user_agent,
        'created_at' => user_action_four.created_at.iso8601(3),
        'updated_at' => user_action_four.updated_at.iso8601(3),
        'acting_user_verification_id' => user_action_four.acting_user_verification_id
      }
    end

    it 'contains the appropriate user action data' do
      get :index

      json_response = JSON.parse(response.body)['data']
      serialized_user_action = json_response.first

      expect(serialized_user_action['id']).to eq(user_action_four.id)
      expect(serialized_user_action['type']).to eq('user_action')
      expect(serialized_user_action['attributes']).to eq(expected_attributes)

      related_user_action_event = serialized_user_action['relationships']['user_action_event']
      expect(related_user_action_event['data']['id']).to eq(user_action_event_two.id.to_s)
      expect(related_user_action_event['data']['type']).to eq('user_action_event')
      expect(related_user_action_event['data']['details']).to eq('Sample event 2')
    end
  end

[bramleyjl]

Similar comments here, we can use more detailed context blocks to explain what the test is doing, and we want to include tests for the absence of pagination arguments as well.

I couldn't find any evidence of what page I was on when making paginated requests; there was no difference in the response body to tell me the page - that should be added so that it can be tested for.

        context 'pagination' do
          let!(:user_action_event) { create(:user_action_event) }

          before do
            11.times do
              create(:user_action, user_action_event:, subject_user_verification_id: user_verification.id)
            end
          end

          context 'when the per_page parameter is not provided' do
            it 'paginates the user actions with a default of 10 per page' do
              get :index

              json_response_data = JSON.parse(response.body)['data']
              json_response_included = JSON.parse(response.body)['included']              

              expect(json_response_data.length).to eq(10)
              expect(json_response_included.first['relationships']['user_actions']['data'].length).to eq(11)
            end
          end

          context 'when the per_page parameter is provided' do
            let (:per_page) { 5 }

            it 'paginates the correct number of user actions per page' do
              get :index, params: { per_page: }

              json_response_data = JSON.parse(response.body)['data']
              json_response_included = JSON.parse(response.body)['included']

              expect(json_response_data.length).to eq(per_page)
              expect(json_response_included.first['relationships']['user_actions']['data'].length).to eq(11)
            end
          end

          context 'when the page parameter is not provided' do
            it 'returns the first page of user actions' do
            end
          end

          context 'when the page parameter is provided' do
            it 'returns the correct page of user actions' do
            end
          end
        end
      end

[bramleyjl]

You can remove this context block since the user action events block it is separated from won't be necessary once the JSON response is fully validated in the other tests.

[bramleyjl]

You can remove this context block with the updated JSON response data validator specs that cover the user action event relationship data.

[bramleyjl]

These should have their created_at times in chronological order for simplicity. There should also be one or more that are older than the default 1.month.ago start_date filter time.

  let!(:user_action_one) do
    create(:user_action, subject_user_verification_id: user_verification.id,
                         user_action_event: user_action_event_one, created_at: 1.year.ago)
  end
  let!(:user_action_two) do
    create(:user_action, subject_user_verification_id: user_verification.id,
                         user_action_event: user_action_event_two, created_at: 2.months.ago)
  end
  let!(:user_action_three) do
    create(:user_action, subject_user_verification_id: user_verification.id,
                         user_action_event: user_action_event_one, created_at: 3.weeks.ago)
  end
  let!(:user_action_four) do
    create(:user_action, subject_user_verification_id: user_verification.id,
                         user_action_event: user_action_event_two, created_at: 4.days.ago)
  end