Fix verification attempts not transactional

[ekzyis]

Description

related to #1818 (comment):

Ideally, this code would not have any races at all, but it's a bit tricky because identifier isn't unique. We could wrap it all in a transaction and do an optimistic lock, or make it seriallizable, but our attempt limit is so low they won't get many more attempts anyway.

This uses SELECT FOR UPDATE with $queryRaw to make sure other tx will wait before reading the attempt count if there's already one tx in progress.

Video

2025-09-19.07-16-20.mp4

Checklist

Are your changes backward compatible? Please answer below:

yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

8, see video

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

n/a

Did you introduce any new environment variables? If so, call them out explicitly here:

no

Did you use AI for this? If so, how much did it assist you?

no

[huumn]

it might be more concise to replace the select with an UPDATE considering that we increment later anyway

[ekzyis]

We might be able to simplify it even more such that only one update (with nested select) and one (conditional) delete is required, see c32298f

Will test more tomorrow, too tired to trust my Q&A rn

[ekzyis]

Q&A of df129b1:

2025-09-19.07-16-20.mp4

I also noticed that we have >9000 rows in our verification_requests table, going all the way back to 2021.

I think we should also only fetch verification requests that haven't expired. Our mail mentions it expires in 5 minutes, but the code doesn't care about that.

[ekzyis]

My code will also leave expired verification requests around but imo, this should be handled in another PR.

Next to #2514, I'd also like to improve the UX if a wrong code was entered because it redirects to a 'try again' page and then you get redirected back if you click it. Ideally we would show the error like a form validation error (but obviously can't validate it on the client).

@Soxasora, in case you feel bad because I have so many suggestions regarding your code, don't, since it's been 7 months, the code has been working well, and it's all just minor stuff that I'm sure you would handle a lot better if you would write this code today. It's completely normal to look back at code and be like "wow, I could have done this a lot better." haha

[Soxasora]

No worries, I didn't feel that way. That code comes from a time in which I was getting to know Prisma better and naively wanted to avoid at all costs pure queries, with all the drawbacks from it.
I got more confident in that area over time, so yes, I look at that code and sigh, lol.

This is what it should've been. Great improvement!

My code will also leave expired verification requests around but imo, this should be handled in another PR.

I contemplated a weekly job for this, to be honest we have lots of places where we never do cleanups. But I did add a midnight cleanup job to comments view tracking just because that could become really big day after day.

I'd also like to improve the UX if a wrong code was entered because it redirects to a 'try again' page and then you get redirected back if you click it. Ideally we would show the error like a form validation error (but obviously can't validate it on the client).

Would be amazing! I didn't consider extra UX while doing that PR as people couldn't access SN via PWA

[Soxasora]

It works correctly! Without any problems. But I left a comment because I'm scratching my head over what seems a double lock?

[ekzyis]

We need to lock on select so other tx also block on select, not after we already updated the value. At least this is my understanding.

I also can't only use UPDATE because I want to update the most recent row and UPDATE can't sort rows before updating

will reply to the other stuff later

[ekzyis]

Ideally we would show the error like a form validation error (but obviously can't validate it on the client).

Would be amazing! I didn't consider extra UX

I imagine this to work similar like the checks for selecting a territory. They run GraphQL queries for validation.

stacker.news/lib/validate.js

Lines 169 to 187 in 21b4c4b

	export function subSelectSchemaMembers (args) {
	return {
	sub: string().required('required').test({
	name: 'sub',
	test: async sub => {
	if (!sub || !sub.length) return false
	return await subExists(sub, args)
	},
	message: 'pick valid territory'
	}).test({
	name: 'sub',
	test: async sub => {
	if (!sub || !sub.length) return false
	return await subActive(sub, args)
	},
	message: 'territory is not active'
	})
	}
	}