Skip to content

Conversation

@uberbrady
Copy link
Member

This is yet another attempt to re-add the ActionType enum in the least invasive way that I can manage. The logaction() method now has a more complex function signature, allowing either a string or an ActionType. If it's a string, it's turned into an ActionType by using the baked-in ::from() method of the backed-enum, which will ensure that only valid ActionTypes are permitted - the ::from() method will throw an error if you use a string that cannot be converted back into an ActionType.

In the process of working with this, I did discover one place that was using request_canceled instead of request canceled so I've also added a migration to fix this in the actionlog table (and fixed the code for this going forward). It's still a little bit scary in that it will iterate through the entire actionlog table, but luckily we're already indexed on the action_type column, so the migration should complete very quickly. I also found a place where we said note_added instead of note added so I fixed that as well.

It's tempting to go through and change every instance where we use logaction() to pass in the appropriate ActionType enum - but that's going to cause too many changes to the code-base, all at once - which we're trying to avoid. This approach still means we get the safety of the Enum, without having to change all of the entire codebase.

@uberbrady uberbrady requested a review from snipe as a code owner November 3, 2025 15:46
@snipe
Copy link
Member

snipe commented Nov 3, 2025

@admmediaconsulting Do you have a particular interest in this? I'm not quite clear how you ended up being a reviewer on this PR.

@snipe snipe added the Merge Ready - on Hold 🤚 Stuff that is fully ready to merge, but on-hold for logistics reasons label Nov 3, 2025
@snipe snipe added this to the November 2025: Weeks 1-2 milestone Nov 3, 2025
Copy link
Collaborator

@marcusmoore marcusmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dig this 👍🏾

Comment on lines +341 to +344
if (is_string($actiontype)) {
$actiontype = ActionType::from($actiontype);
}
$this->action_type = $actiontype->value;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that we're keeping the existing behavior around (passing in a string) while also checking that the string is a valid type using ::from 👍🏾

@snipe snipe merged commit 21baea2 into grokability:develop Nov 10, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge Ready - on Hold 🤚 Stuff that is fully ready to merge, but on-hold for logistics reasons

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants