[1003] Add more feature tests

[goldsmithb]

Jira Ticket

---

This PR adds more feature tests to AC, focusing on the features listed in the top AC features to test document that was compiled by Eric, Jack, and myself.

• That google doc also lists the feature tests that are currently planned and those that have been implemented in this PR.

Many of the tests focus on the admin-only interface UI interactions.

[JackBlackLight]

This is so great to have all these tests! Thanks for doing this @goldsmithb. I added a couple minor questions/comments but these are good to go.

[JackBlackLight]

This is a matter of preference, but when I saw 'testid', I thought it might have been left in the code by mistake. I see that at present it is being used by a test and that's why it has this name, which makes sense, but I think I'll be confused by it each time i see it :)

It could conceivably be used outside of a test at some future point, right? Would it be bad to rename it as just 'id'?

[JackBlackLight]

See rationale above - could this become 'data-id'?

[JackBlackLight]

I'm curious about the challenge you're encountering here. Don't know if I can help, but would be happy to talk it over.

[goldsmithb]

Thanks for the review @JackBlackLight ! I will respond to your comments here.

For the data-testids, I could see reasons to go either way. I used them here because there were no convenient Capybara selectors I could find for writing certain browser-driven tests that verify the presence of these elements. Currently, they aren't used by any JS code or style rules (and also don't need to follow uniqueness requirement for normal HTML ids), but you are right that we could always use them for more/more tests in the future, and there isn't much of a practical difference. In past codebases I have worked where these were used more extensively for this type of testing, we had a step in our deployment process that removed all the data-test attributes from the source, because there was a concern that leaving them in could expose our test surface to malicious users (though I'm not sure I find this entirely convincing -- because I only include them when there isn't a more convenient way to test these elements, it doesn't really expose much anyways). I think it is worth converting them to normal IDs, specially because we don't use them extensively.

For the admin-only item tests, I'm not sure what is causing the trouble, but it seems that elements that should render in an admin-user context are not being selected properly in the Capybara tests. Sometimes they work and sometimes they don't, and I haven't been able to get them to pass consistently in the Github CI environment. These are as you know tricky to diagnose, so I would love to take a look with you when you are back. I will leave them as skipped for now.

[goldsmithb]

For some reason, this wasn't picked up as merged, but I rebased and merged this into main last week.

Commit is here: 3b107da4aab21aef2d3df44d089176b2a3a0b918

Changes were deployed with v5.0.4