<blockquote>
<p>Meanwhile maybe <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=360803" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/gravitystorm">@gravitystorm</a> will let us know what he had in mind.</p>
</blockquote>
<p>It was just a general point. Moving complex logic to models makes it easier to test, among other things. For example, it's easier to check that the code selects the correct objects from the database in a model test, rather than having to parse the XML output of a controller to check if the correct objects were retrieved (and e.g. not extra objects).</p>
<p>Tom's suggestion of using scopes to abstract away the complexity from the controller is the kind of thing that I had in mind. The scopes are a good way to ensure that we don't end up making complex queries in multiple places, as well as being a natural home for custom Tracepoint finder methods that go being straightforward <code>find ... order ... limit</code> that you normally get in controllers.</p>
<p>Anyway, it was more of a "if you're working on this then moving the code would help" rather than any major problem.</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/2068#issuecomment-440253152">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLWPjuGiZW7L6Kk3-wj0DH1S4Urroks5uw_KSgaJpZM4YpjGw">mute the thread</a>.<img src="https://github.com/notifications/beacon/ABWnLb-Qj_dTCfjT2a50Mcby3e5tmKULks5uw_KSgaJpZM4YpjGw.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@gravitystorm in #2068: \u003e Meanwhile maybe @gravitystorm will let us know what he had in mind.\r\n\r\nIt was just a general point. Moving complex logic to models makes it easier to test, among other things. For example, it's easier to check that the code selects the correct objects from the database in a model test, rather than having to parse the XML output of a controller to check if the correct objects were retrieved (and e.g. not extra objects).\r\n\r\nTom's suggestion of using scopes to abstract away the complexity from the controller is the kind of thing that I had in mind. The scopes are a good way to ensure that we don't end up making complex queries in multiple places, as well as being a natural home for custom Tracepoint finder methods that go being straightforward `find ... order ... limit` that you normally get in controllers.\r\n\r\nAnyway, it was more of a \"if you're working on this then moving the code would help\" rather than any major problem.\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/2068#issuecomment-440253152"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/2068#issuecomment-440253152",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/2068#issuecomment-440253152",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
},
{
"@type": "MessageCard",
"@context": "http://schema.org/extensions",
"hideOriginalBody": "false",
"originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB",
"title": "Re: [openstreetmap/openstreetmap-website] Move tracepoint ordering logic into tracepoint model, and add tests (#2068)",
"sections": [
{
"text": "",
"activityTitle": "**Andy Allan**",
"activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png",
"activitySubtitle": "@gravitystorm",
"facts": [

]
}
],
"potentialAction": [
{
"name": "Add a comment",
"@type": "ActionCard",
"inputs": [
{
"isMultiLine": true,
"@type": "TextInput",
"id": "IssueComment",
"isRequired": false
}
],
"actions": [
{
"name": "Comment",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"IssueComment\",\n\"repositoryFullName\": \"openstreetmap/openstreetmap-website\",\n\"issueId\": 2068,\n\"IssueComment\": \"{{IssueComment.value}}\"\n}"
}
]
},
{
"name": "Close pull request",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"PullRequestClose\",\n\"repositoryFullName\": \"openstreetmap/openstreetmap-website\",\n\"pullRequestId\": 2068\n}"
},
{
"targets": [
{
"os": "default",
"uri": "https://github.com/openstreetmap/openstreetmap-website/pull/2068#issuecomment-440253152"
}
],
"@type": "OpenUri",
"name": "View on GitHub"
},
{
"name": "Unsubscribe",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 413544880\n}"
}
],
"themeColor": "26292E"
}
]</script>