<p></p>
<p>First of all, I'm sorry that it has taken so long to respond here.</p>
<p>I've reviewed this PR a couple of times over the last few months, and mulled over the options, and there's a number of issues with it, including:</p>
<ul>
<li>Tests are added for both <code>title</code> and <code>description</code>, but the migration only covers the <code>title</code>.</li>
<li>The model tests can be simplified by using e.g. <code>redaction.valid?</code> instead of attempting to save and then asserting against the errors.</li>
<li>A lot of comments are added to existing tests, but they mostly just restate exactly what the code already says.</li>
<li>Due to changes in the meantime, this PR now conflicts with master.</li>
</ul>
<p>But the big thing is that we've introduced the 'strong_migrations' gem, which flags up this migration as potentially locking the database while it runs. So the migration will have to be substantially re-written to deal with this.</p>
<p>My recommendation for reworking this PR is to split it into two parts - one for the additional model tests (which only test the model validations anyway, not the database constraints) and a separate one for the migration, rewritten to be compatible with <code>strong_migrations</code>.</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2034#issuecomment-693298705">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLJUQTTFZR4L4RRYSI3SGCC47ANCNFSM4F64KF3Q">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AAK2OLLRWQHAKY7BPGT6HALSGCC47A5CNFSM4F64KF32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFFJOMEI.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/2034#issuecomment-693298705",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/2034#issuecomment-693298705",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>