Thank you for the insight, I updated the PR 
What do you think we should work on next? I’ll continue to work on existing PRs if changes are requested, and the admin pages require a bit of testing and styling. What should I be focusing on now, and what should we be working on after this is complete?
Once you get the conflicts resolved in the outstanding PRs, let’s ensure we have head of 3.x running on atlas-stg and evaluate it. I think you’ve got it pretty close to where we could deploy to production. Deploying to production will take several steps and some coordination with @cintiadr :
git checkout 3.x
git merge --strategy=ours --no-commit master
git commit -m "atlas 3.1 - replacing php with node"
git checkout master
git merge 3.x
git push
I resolved the conflicts, but it looks like ATLAS-152 and ATLAS-153 aren’t working as expected after testing them using the newly merged API. I’ll let you know soon as I fix them, so that they can be merged.
ATLAS-152, and ATLAS-153 are now fixed and are working properly . ATLAS-151 (which was merged) continues to be broken. I’ll create a new ticket and make a PR to fix it.
@burke @harsha89 @cintiadr The user interface to manage marker types has been fixed (ATLAS-156)!
Also, here’s the updated Atlas Server Rest API.
Shall I deploy this version to production this weekend?
Great progress, @heliostrike! I’ve merged PRs and deployed to staging. There appears to be a bug in how versions are handled.
Take a look at these lines in atlas.js:
if (site.openmrs_version && site.openmrs_version !== "" && site.openmrs_version !== "unknown") {
var version = site.openmrs_version;
return version.match(/\d+(\.\d+)/g).toString();
}
This code assumes versions are blank, the literal value “unknown” or in the form of 1-2 numbers separated with a period. This doesn’t support our versioning format and fails if the version is “Other” or “Unknown”.
If we can get it working. 
@heliostrike, to give you a feel for the (endless) number of additional features we’d like to see, I added some tickets to the Atlas project:
None of these are required to get Atlas 3.x deployed to production as Atlas 3.1. So, let’s concentrate on knocking out any existing bugs and – once we’re confident atlas-stg is behaving properly – then going through the steps I mentioned earlier to convert the 3.x into the new master branch and tag it as 3.1 so it can be deployed as the new production OpenMRS Atlas!
@burke I was wondering why atlas-stg had no markers! How should we handle the ‘Other’ case?
if(site.openmrs_version == "Other") return site.openmrs_version;
or should I return null? Looking at how Atlas operates, I think we should return ‘Other’.
Oh. I misspoke. That code isn’t validating that the version is only 1-2 numbers separated by a period… it’s extracting the major & minor numbers from a version number (returning “1.2” if given “1.2.3”) or returning null. How about something like this?
function versionMajMinForSite(site) {
// Fetch major & minor version of site's OpenMRS version; otherwise, return null
var majorMinorVersion = site.openmrs_version ? site.openmrs_version.match(/\d+(\.\d+)/g) : null;
return majorMinorVersion ? majorMinorVersion.toString() : null;
}
@burke I just made a PR that does almost the same thing. Please have a look at it.
Updated the PR to do the same thing
Deploying it to production requires changes to both ansible and applying changes to the database. Tagging won’t be enough.
For ATLAS-164, would you suggest using the existing PATCH route (which needs to be given all the marker details), or creating a new ping route (PATCH?) to update the date given the marker id?
A good RESTful API works with resources (nouns) and not methods (verbs). Since “ping” is a method/action, it doesn’t fit in a RESTful API. While you can approach this using reification - e.g., creating a MarkerUpdate resource that can track the updating of a marker – that’s more useful for long-acting or multi-step workflows.
I would suggest simply supporting an empty body to PATCH /markers/:id (i.e., sending either nothing or an empty object { }) as a mechanism to simply change the date changed (would need to be authenticated and have edit rights to the marker, of course). Since the response should be the updated resource, it would give the client exactly what it needs to update the resource locally.
Yes. Thanks for pointing this out. I was assuming @heliostrike would need to coordinate with you. I was just outlining the steps he should be taking so you & he have a tagged version of master from which to deploy.
@heliostrike, I merged most of your PRs and put some comments for you on the remaining two. When I deployed to staging, I noticed a bug when saving sites. Some sites seemed to save okay, but when I tried saving Alexis Duque’s Site (in Libya), I got an error: “Error saving your maker - Please try again! - Internal Server Error.”
Looking at the request, it’s a PATCH to /marker/:id, but instead of having the JSON as the body, it looked like the JSON was being coerced into the key for a key-value pair. For example, instead of sending:
{
"foo": "bar"
}
in the body of the PATCH request, the body was in the form:
{"foo":"bar"}:
This is the format of HTTP form data. Looking closer, I notice the request has the headers
Accept: text/plain
Content-Type: application/x-www-form-urlencoded; charset=UTF-8
when I would expect these headers to be:
Accept: application/json
Content-Type: application/json
I also think this explains this ugly little hack that slipped into PR #45.
All requests that expect to receive json should have an “Accept: application/json” header and any requests sending json should include a “Content-type: application/json” header. This would be a good opportunity to scan all ajax requests in the client to make sure they are setting these headers appropriately.
I created a ticket for this bug:
I think it may be because we’re passing data as text through AJAX.
$.ajax({
//If marker id is a uuid, its an PATCH update request to '/marker/[id]'
//else it is an POST create request to '/marker'
url: "/marker" + (isUpdateRequest ? "/"+id : ""),
type: isUpdateRequest?"PATCH":"POST",
data: json,
dataType: "text", // <-- over here!!!
})
It existed in the codebase for a while and I sort of went with it whenever I created AJAX calls in the future, so the bug is sort of my fault :p. This led to the following line in a lot of node routes:
req.body = JSON.parse(Object.keys(req.body)[0]);
//Get data from the squished key
The following line was mostly used to remove the array brackets (’[ ]’) surronding the response:
response = response.substring(1,response.length-1);
I’ll start working on converting those text calls to json ones tomorrow (thats in 8 hours). Where can I view the server logs, to assist me while testing? (I couldn’t find anything on the browser’s console, so I’m assuming that the error you spotted was on some server logs)
Also, I updated ATLAS-164. Please have a look at it when possible
I use Chrome’s developer tools.
You’ll see the requests in the Network tool, including the call to /marker/:id. Select that and you’ll see the details on the right, where you can browse through the request & response headers along with content sent or received. Let me know if you have any problems finding it and we can find time to pair up and I can walk you through it.
Thank you, I found it
I’ll start working on the issue. Also updated ATLAS-164 again.
I corrected the headers - ATLAS-172. I think ATLAS-165 can be solved by sending the changed_date through the response. What do you think?
Awesome! Can you add IDs to log entries? I’m hoping it’s just a tiny tweak to your log entries… just thought it would be good to address that in this PR (where you’re adding them) rather than creating another PR for it.
Exactly! The REST response of adding or updating a resource should contain the resource that was created or updated (which would include the date_changed in this case). That’s what I was referring to when I said “Since the response should be the updated resource, it would give the client exactly what it needs to update the resource locally.” above.
Is it ok to get the version/type/distribution names (for the logs) from the frontend? Getting it from the backend would need another SQL query. 
If that’s alright, then the changes are made.