<p></p>
<p dir="auto">I worked on the validation topic today, but I don't have something that I'm happy with.</p>
<p dir="auto">I thought of two approaches. One is to write custom validation code in <code class="notranslate">ResourceBackend.load</code>, e.g. that uses a regexp to check the URLs are OK. But then when I started choosing schemes, and doing blank handling, then this feels a bit cludgy since we normally put attribute validation into the models. Reinventing validations feels wrong. If we can declare the validation in the model, then we can do a normal <code class="notranslate">validates...</code> declaration instead. So I tried that approach instead, and used the <code class="notranslate">validate_url</code> gem too. You can see my commit at <a class="commit-link" data-hovercard-type="commit" data-hovercard-url="https://github.com/gravitystorm/openstreetmap-website/commit/7b5df0daaf7b5a230f180ec914b02db66bfa2fee/hovercard" href="https://github.com/gravitystorm/openstreetmap-website/commit/7b5df0daaf7b5a230f180ec914b02db66bfa2fee">gravitystorm@<tt>7b5df0d</tt></a> which I haven't added to this PR.</p>
<p dir="auto">This threw up some issues:</p>
<ul dir="auto">
<li>FrozenRecord doesn't have anything where it checks validity, nor can I see any way to crowbar that into the loading of each record. So instead I fail loudly during object initialization if there's an invalid URL. This should be fine, in theory, since we'll only update to versions of the node package from time to time, and if there's one with invalid urls then I think it's fine for the tests to fail (and the app to not boot).</li>
<li>OCI already contains various schemes, not just http and https but also mailto and xmpp. I wasn't expecting that.</li>
<li>OCI also contains invalid urls <g-emoji class="g-emoji" alias="facepalm" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f926.png">🤦</g-emoji> For example, <code class="notranslate">https://matrix.to/#/#openstreetmap-cat:matrix.org</code> is <a href="https://stackoverflow.com/a/26119120/105451" rel="nofollow">invalid</a> (albeit accepted by browsers) since the <code class="notranslate">#</code> isn't permitted inside fragments.</li>
</ul>
<p dir="auto">The last of these is a bit of a hard fail for this approach, since we have to deal with what OCI gives us, and this is not a malicious URL and failing to handle it gracefully is not reasonable.</p>
<p dir="auto">So trying to do things "the right way" hasn't worked as well as I hoped. If anyone has any suggestions to build on this approach then I'm all ears, otherwise my next attempt will be to "reinvent validations" by hand in the <code class="notranslate">.load</code> method (sigh).</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/3301#issuecomment-1292454360">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLK44N3X42KZUCRHK2LWFF4AJANCNFSM5CXUIRLQ">unsubscribe</a>.<br />You are receiving this because you are subscribed to this thread.<img src="https://github.com/notifications/beacon/AAK2OLP2V5GVUDL2JMZM4NLWFF4AJA5CNFSM5CXUIRL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOJUEUTWA.gif" height="1" width="1" alt="" /><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openstreetmap/openstreetmap-website/pull/3301/c1292454360</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/3301#issuecomment-1292454360",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/3301#issuecomment-1292454360",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>