The following discussions from !107970 should be addressed: - [ ] @jessieay started a [discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107970#note_1247927372): > **Question**: more for my own learning than anything else: do we have an existing dev docs on using time in Gitlab/Ruby? > > I ask because the use of `utc` here does not change the value because we are using if `gitlab.yml` has the default time zone of `UTC`: > > ``` > > Time.current.beginning_of_month > => Sun, 01 Jan 2023 00:00:00.000000000 UTC +00:00 > > Time.current.utc.beginning_of_month > => 2023-01-01 00:00:00 UTC > ``` > > But this is of course configurable, so it makes a lot of sense to explicitly request UTC time. If this is not already documented somewhere then it would be great to write it down! I am also interested in any other timezone-related quirks that might exist in the codebase. - [ ] @jessieay started a [discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107970#note_1247927375): > **Suggestion**: Remove this method and move its logic directly into `.current_month` > > Reasoning: > > This method is only used in the context of the `current_month` scope, so it really should be considered a private method. > > At the same time, [making a class method private in Ruby](https://jakeyesbeck.com/2016/01/24/ruby-private-class-methods/) always feels kind of hacky to me. > > Because the method is so simple, why not include its logic directly in the `current_month` scope itself? I think it will be just as readable if we do that and will result in less code :tada: - [ ] @jessieay started a [discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107970#note_1247927385): > **Suggestion (non-blocking)**: add a validation for this uniqueness constraint and test that instead > > Reasoning: > > I do not usually see RSpec tests for database constraints. I think this is because I almost always [add ActiveRecord validations to mirror database uniqueness constraints](https://thoughtbot.com/blog/validation-database-constraint-or-both) so that [users see a validation error rather than an application error](https://thoughtbot.com/blog/the-perils-of-uniqueness-validations). > > In this case, I imagine that the data transfer will *not* be reated by a human, so perhaps the human readable-ness of a validation message is not required and we do want an application error raised. So really up to you as subject-matter expert to decide whether we should do this or not...