Backport new features in v2 registrations to v3

[dunkOnIT]

No description provided.

[FinnIckler]

you got your changes from the other PR in here

[dunkOnIT]

you got your changes from the other PR in here

This is deliberate although now that I'm thinking about it probably unnecessary

[dunkOnIT]

Test failures are due to some very frustrating quirks of how competitions with no competitor limit appear to be handled in the monolith

[gregorbg]

Test failures are due to some very frustrating quirks of how competitions with no competitor limit appear to be handled in the monolith

Can you be a bit more specific? Do you need help with figuring these quirks out?

[dunkOnIT]

new_status == 'accepted' && competition.competitor_limit > 0 && Registration.accepted.count >= competition.competitor_limit

This line compares competitor_limit to 0. In the microservice, this was fine as it came through as 0 in the JSON payload. In the monolith's db it is also stored as 0*, so I think the comparison with 0 is the right thing to do.

However, when trying to set competitor_limit { 0 } in the competition factory, we hit validation errors in the competition model which say that it can't be 0.

I see a few solutions here - probably most partial to 3 for its simplicity and the fact that it will align with what the actual values in the database are, but I don't have enough understanding to really know if it's an acceptable solution or not:

1. Change the competitor_limit > 0 comparison to support the case where competitor_limit is nil [I really dislike this - we shouldn't change perfectly fine production code because of issues with test data]
2. Figure out where and how competitor_limit is being changed from nil to 0 - because it appears that somewhere it is? - and use that to inform what the best course of action is [I haven't been able to find it/figure this out]
3. Change the validation to allow 0 if competitor_limit_enabled is false
4. Skip validations when creating competition factory objects where event limit isn't enabled [Technically possible, but not a fun way to handle things, especially because the default Factory setting is false for competitor_limit_enabled - and changing that is likely to cause a number of other tests which rely on this assumption to fail

Open to other solutions here, though!

*I have confirmed this using the Rails console and dbeaver for TexasFMCChampionship2024.

[gregorbg]

Sidenote: Unless I'm missing something completely obvious, aren't some of these changes the same as the other PR you just merged?

[dunkOnIT]

I branched off the other PR - have just merged main in, so the diff should just be the new changes now

[gregorbg]

Ah, now I can track what's going on much easier, and the problem you described is also clearly visible in the code now :)

[gregorbg]

I can confirm that TexasFMCChampionship2024 has a competitor limit of 0 and that indeed, there's 0 written to the database on prod as well. So your basic line of thought should be "Hmm, somehow we have valid models with 0 in prod, so how can that be in the existing codebase?"

The answer lies in checking the actual validations that fail: competition.rb has (among a gazillion others) the following validation:

validates_numericality_of :competitor_limit, greater_than_or_equal_to: 1, less_than_or_equal_to: MAX_COMPETITOR_LIMIT, if: :competitor_limit_enabled?

Note the if: part at the end. So the "trick" here is to not just set competitor_limit { 0 } in the factory, but (possibly through use of a trait) also set competitor_limit_enabled { false } at the same time.