Ban `attributes_for` for migration specs

What does this MR do?

Ban attributes_for for migration specs. Rewriting issue: #299420

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

• Changelog entry
• Documentation (if required)
• Code review guidelines
• Merge request performance guidelines
• Style guides
• Database guides
• Separation of EE specific content

Availability and Testing

• Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
• Tested in all supported browsers
• Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

• Label as security and @ mention @gitlab-com/gl-security/appsec
• The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
• Security reports checked/validated by a reviewer from the AppSec team

Related to #299410 (closed)

changed milestone to %13.9

added static code analysis test typefeature typemaintenance labels

added 1 commit

• 646793ea - Ban attributes_for for migration specs

Compare with previous version

added backend label

1 Message

CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, run the following: bin/changelog -m 52182 "Ban \`attributes_for\` for migration specs" If you want to create a changelog entry for GitLab EE, run the following instead: bin/changelog --ee -m 52182 "Ban \`attributes_for\` for migration specs" If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

Reviewer roulette

Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.

To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

Once you've decided who will review this merge request, mention them as you normally would! Danger does not automatically notify them for you.

Category	Reviewer	Maintainer
backend	Reuben Pereira (@rpereira2) (UTC+5.5, 2.5 hours behind @godfat-gitlab)	Michael Kozono (@mkozono) (UTC-8, 16 hours behind @godfat-gitlab)

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

added 1 commit

• c33e974a - Ban attributes_for for migration specs

Compare with previous version

added 17 commits

• c33e974a...ba4bac42 - 16 commits from branch master
• ab85b805 - Ban attributes_for for migration specs

Compare with previous version

mentioned in issue #299420

changed the description

@jprovaznik Please review? Thanks!

assigned to @jprovaznik and unassigned @godfat-gitlab

resolved all threads

approved this merge request

Thanks @godfat-gitlab, LGTM , MWPS set

enabled an automatic merge when the pipeline for 4f3fdfe7 succeeds

merged

mentioned in commit fd8ae0f6

mentioned in commit 5b8bc9c2

mentioned in commit 5f10ae68

mentioned in merge request !52233 (merged)

mentioned in issue #299410 (closed)

added workflowstaging label

added workflowcanary label and removed workflowstaging label

added workflowproduction label and removed workflowcanary label

mentioned in commit c3680461

mentioned in merge request !52492 (merged)

mentioned in commit 21d87c2c

mentioned in merge request !52496 (merged)

mentioned in commit 4a6f0c5f

mentioned in merge request !52497 (merged)

mentioned in commit c0288539

mentioned in merge request !52498 (merged)

mentioned in commit 20fb8b08

mentioned in commit d5a5f82e

mentioned in commit 4f6882dc

mentioned in commit d2a2b6ea

@godfat-gitlab I want to ask why attributes_for is considered bad? I can understand that using factory to create records can be deceiving, but attributes_for just returns a hash for use. It avoids repeatedly making up dummy values.

resolved all threads

mentioned in commit fa76a8d8

mentioned in commit 794c8098

mentioned in commit ccecedde

mentioned in merge request !53372 (merged)

mentioned in commit 0778090e

mentioned in merge request !53374 (merged)

mentioned in commit 8c6ac801

mentioned in merge request !53375 (merged)

mentioned in commit 28e11e6d

mentioned in merge request !53381 (merged)

mentioned in commit 35f82ea5

mentioned in commit 36e32d9c

mentioned in commit 9dbf6fbe

mentioned in commit 438ec458

mentioned in merge request !53614 (merged)

mentioned in commit ea67a91f

mentioned in commit 18d5d5fe

mentioned in commit 61d1606d

added releasedcandidate label

removed typefeature label