Follow-up from "Implement additional logging when expiring orders"
Problem
In !2915 a discussion (+2 comments) about why there is a #to_param
and #to_i
being called on a parameter (here) that is only passed in as an Integer (here). It turned out that this was recently changed in https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/2303#note_467690097 to meet Sidekiq's best practices. Any remaining jobs during the deploy had to be supported then and therefore this was still needed.
Proposal
Since there is only one usage where an Integer is being passed in and all jobs the ExpireOrderJob
class are processed (no retires left as well), we can remove the #to_param
and #to_i
. The #to_i
is also not needed since .find_by_id
handles this anyway:
[2] pry(main)> Order.find_by_id(1)
Order Load (0.4ms) SELECT "orders".* FROM "orders" WHERE "orders"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]
[3] pry(main)> Order.find_by_id(nil)
Order Load (0.3ms) SELECT "orders".* FROM "orders" WHERE "orders"."id" IS NULL LIMIT $1 [["LIMIT", 1]]
[4] pry(main)> Order.find_by_id("")
Order Load (0.4ms) SELECT "orders".* FROM "orders" WHERE "orders"."id" = $1 LIMIT $2 [["id", nil], ["LIMIT", 1]]
[5] pry(main)> Order.find_by_id("a-string")
Order Load (0.4ms) SELECT "orders".* FROM "orders" WHERE "orders"."id" = $1 LIMIT $2 [["id", 0], ["LIMIT", 1]]
[6] pry(main)> Order.find_by_id("1")
Order Load (0.4ms) SELECT "orders".* FROM "orders" WHERE "orders"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]
Result
Remove unnecessary method calls and therefore clean up the code and avoid possible confusion.