LG-9334: Clean up Funnel::DocAuth::RegisterStep#8272
Conversation
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM
It should be safe to remove the ignored column values in this PR... we messed that up last time, so if we want to be extra careful we could do that in a follow-up PR
Gemfile.lock
Outdated
There was a problem hiding this comment.
do we need to make sure we as a team are all on the same patch? I would back this change out for now
There was a problem hiding this comment.
I don't know, but ruby-build doesn't seem to offer any patched versions of 3.2.2 unless I'm missing something. I restored it in d863930ae, but I'm going to see this every time...
There was a problem hiding this comment.
Do we want to delete link_sent? The data isn't used, but it's still being written to via the FSM LinkSentStep. I would wait on this until that step is pulled out of the Flow State Machine. I'm surprised that didn't break any tests.
There was a problem hiding this comment.
Gotcha - restored in 4b58deaca
Lint won't pass without removing the ignored columns |
Yeah, I'm happy to wait to drop the ignored columns for a follow on PR and have someone with higher permissions force the merge. |
I don't think that would be a good idea, because that means every PR merged after this one would also need a force merge. I think the right thing to do is to remove those columns. Alternatively, we could disable |
Yes, that would be a bad idea to force merge! 🙃 I disabled the cop in 4e5a3ff8e and will prep a separate PR to remove them once this PR is in production. |
db/schema.rb
Outdated
There was a problem hiding this comment.
did this get re-added? it's not in the migration?
There was a problem hiding this comment.
oh I see now, it got re-ordered? how did that happen, and where did its default/precision go?
There was a problem hiding this comment.
Not sure but I rebuilt the database from main, then applied the new migration and it put it back as it had been so I'm guessing it was due to restoring it by rolling back the migration which didn't include the precision/default. Fixed in 16f53bbb9
[skip changelog] Removes unused columns from the doc_auth_logs table: - send_link_view_at - send_link_view_count - email_sent_view_at - email_sent_view_count Co-authored-by: John Maxwell <john.maxwell@gsa.gov>
solipet
force-pushed
the
dprice-lg-9334-clean-up-register-step
branch
from
16f53bb to
611a0f4
Compare
4 participants
Removes unused and ignored columns from the doc_auth_logs table: