GSOC 2019 : OpenMRS Atlas 3.1

Tags: #<Tag:0x00007f0769ab3660>

@cintiadr I think I get it now!

When I last tested it, clicking the button just zoomed into the last added site. I assume that ‘auth_site’ is the list of sites owned by the logged in user. The code contains a small bug that’s causing auth_site to be set to null. So creating a new site leaves auth_site with an array containing a single site, so that’s where it zooms in when the button is clicked.

I think repeatedly clicking the button should iteratively zoom into sites belonging to the logged in user, that is what I could infer from the code. :slight_smile:

Maybe a better design would be replace ‘edit my site’ by ‘show only my sites’.

That might be easier to understand (and wouldn’t be hard to implement?)

1 Like

@cintiadr I agree! I’m currently working on getting the basic functionality to work, like adding, editing, and deleting markers. (Unfortunately there are a lot of (rather deliberate)bugs here, and messed up variable-naming, so I estimate that this may take a couple’ weeks). Once that’s done I’ll start working on the UI.

@burke @harsha89 @cintiadr

Here’s a brief update of what I’ve been doing.

  • Add, delete, and edit routes are sort of working.
  • There’s a small issue with add/edit where the distribution isn’t getting saved.
  • As I was recording my screen to log what I’ve done in the last few days, delete and edit didn’t work in the particular session.
  • Fixed the ‘Edit my Site’ button.

Stuff to do:

  • Fix whatever’s wrong with the add/edit/delete.
  • Fix a bunch of UI related issues, but I think I’ll have to do it over time.
  • Clean and comment the code a little.
1 Like

@burke @harsha89 @cintiadr

Add, edit, and delete are now working! Here’s a video of the newly implemented functionality.

Day’s progress:

  • When I implemented edit yesterday, turns out the code created a new marker in the same location, which caused made me believe that something was wrong with the delete route, as a marker would remain, even after deleting the marker on top. But that’s fixed now. :slight_smile:
  • Fixing the above broke the edit functionality by inducing a silly bug. It took me quite a lot of time to spot that.
  • Fixed a bug which recolored all markers to blue on editing a marker.

Next Steps:

  • I commented a few lines that caused UI bugs, but it was probably there for a reason, so I should look into that.
  • Check out what’s wrong with the Atlas Module.

Atlas-stg is back!

https://atlas-stg.openmrs.org/

Latest code, with ldap login, deployed. I added permission to our ldap-stg to @heliostrike, @burke, @harsha89 and myself. Login seems to work fine!

@heliostrike I needed to change the config a little to get it to work though.

2 Likes

@burke @harsha89 @cintiadr Since the atlas webpage is sort of working, I plan to work on cleaning up atlas module this week. There are a lot of post requests being called in the module which we no longer need, as stuff happens through the webpage. Simply changing the server url in AtlasConstants, brings up Atlas 3.1 onto the iframe, but I feel like there may be a lot more code running in the background than we want to.

I’d like to simultaneously work on the open PR if it demands any modifications. Is that ok?

1 Like

Woohoo! Great work, @heliostrike! :partying_face:

Getting LDAP working is an excellent start. There are still some basic features needed or that we need to verify are working for the server before we can go from staging to production.

  • Document the REST API for Atlas. This doesn’t have to be fancy; simply making a wiki page that enumerates the API functions would be fine. Rather than simply fixing what’s broken, I’d like to have a shared understanding of the API we want to have and then work to make it happen. Would you mind making a child wiki page under your project page for the Atlas REST API and describe your current understanding of the API? We could iterate from there.
  • Authentication should include the authenticated OpenMRS ID (for the REST API to know when a user has edit access for a given marker) and whether or not the user is an admin. While it would be nice to allow multiple users to edit a marker or for someone to delegate others, I don’t think we need to get much fancier with RBAC (some is either anonymous, authenticated user, authenticated admin).
  • Avoid leaking marker UUIDs. Assuming we continue to use marker UUIDs as a shared secret for updates, we need to ensure we aren’t returning UUIDs to someone who isn’t an owner of the marker or an admin.
  • Continue to support legacy /ping.php URL for updates. We should continue to accept a post to /ping.php from old versions of the module for backwards compatibility. Even if we update the module, old versions of the module could be running around the world for years to come and it would be nice to accept their pings.

Also, we should make an effort to work from ATLAS tickets – i.e., all tasks you are taking should be reflected in a tickets, so PRs and commits can reference the ticket they are addressing. I think you are already doing this. Maybe just need to remember to prefix PRs with the ticket – e.g., “ATLAS-133: Fixed add, delete, and edit markers functionality”.

1 Like

@burke I agree. I wrongly assumed that displaying the atlas webpage was all that the atlas module was doing, by looking at it. But digging into the code shows more than that, so I think that the task will have to be postponed to a later date.

Where would you like me post the documentation of the REST API? Should I also document the API in the PHP code?

Atlassian Confluence (our wiki software) arranges pages in hierarchies and, by default, any page you create becomes a child of the page you are viewing when you create the new page. So, while viewing your GSoC project page, click the Create button at the top to create a child page with title like “Atlas Server REST API”.

You’ll have basic CRUD for /marker, but also calls for getting all markers, getting types/versions/distros, etc. We’ll also have endpoints for managing types/versions/distros along with administrative features.

It doesn’t have to be fancy and we can iterate on it over time. Just enumerate the API calls you think we have or should have in the API. Once we agree on what we want the API to be, it will be easier for you to ticket & tackle. :slight_smile:

No. All we really care about is the legacy format of the /ping.php function used for reporting updates from the legacy module. We should plan to continue to support that URL as a deprecated endpoint until we can document that it is not being used (e.g., if we go 3-6 months without a single call to it from an OpenMRS module, we can remove it).

1 Like

Here’s the list of API calls we have currently. The calls under ping.php are the ones being used in the atlas module (I’ll start working on implementing them).

I’ll try to write some code that adds an admin flag to authenticated users, so that we can move on to implementing admin calls.

@burke I’ve updated the PR with the calls made by the atlas module. I verified that they work, using Postman. However, since we don’t have a way to check whether a call was actually made by the atlas module, I’ve gated the routes with ‘isAuthenticated’ to make them unusable for the time being.

Thanks @heliostrike.

So, it looks like UUID is associated with the user and not the individual marker? Something seems amiss there. When a module is used to update a marker, it is associated with a single marker (I thought by knowing its UUID).

What do you think about properly using HTTP verbs (e.g., PUT & PATCH for replacing and updating a resource, respectively)?

@burke Each marker has a different UUID. I’m assuming you’re looking at ‘GET /marker/uid/{uid}’. iIt fetches markers that were created by user with username ‘uid’ (user ID, but yea… I think I should change the call to something less confusing… maybe ‘/marker/created_by/{username}’?).

I haven’t used PUT and PATCH much so I have no idea. I’ll read up on it. :slight_smile:

@burke I wrote some code that stores whether the user is admin or not, in the user session object.

Also, yup, the OpenMRS ID is stored in the session for validation purposes. :slight_smile:

@burke @harsha89 @cintiadr

  • Document the REST API for Atlas Here’s the page,

  • Authentication Here’s the JIRA issue and PR.

  • Avoid leaking marker UUIDs I added a function called ‘filterMarkerIds’ in this PR, which replaces marker ids of markers that aren’t owned by the user.

  • Continue to support legacy /ping.php URL for updates Its in this PR, but I gated them with ‘isAuthenticated’ to make them unusable for the time being, until we figure out some way for the atlas module to authenticate.

What do you think I should work on next?

How about GET /markers?username={OpenMRS ID} to return a list of marker resources the given user can edit? (FYI – for now, it may only be the creator; in the future, we may add the ability to delegate edit access to others, so multiple people might be able to maintain a marker).

I think the exception would be for aminds. We wouldn’t want to return all markers (even though an admin could edit any marker); rather, we’d return those markers that the admin is specifically named as owner (or delegate).

We’ll need GET /types and GET /versions if we don’t have those yet. We may also need at least POST for types, versions, and distributions so an admin can manage the available values.

Using ids (UUIDs) as a “secret” feels like a poor design decision now. The goal was to ensure a module could send updates without having to be authenticated; however, it would probably be better to have an explicit “update token” distinct from a marker’s id that, if provided, would allow for unauthenticated updates of the marker. Then we wouldn’t have to worry about filtering ids, we could more easily support future read-only calls for specific markers (for reporting, etc.), and we’d never send the update token to clients. The legacy /ping.php endpoint could look up the marker by update token.

1 Like

Sounds good to me. :slight_smile:

It looks like there’s no single place whether types and versions are being stored. Marker types are being hardcoded in code multiple times, so I think we’ll have to store it in the database. As for versions, it isn’t being used much in the code, but I assume it isn’t more than a string.

What do you think is a good database schema for them? After seeing its usages, I can imagine them to be the following.

types(id, name, icon) Example: (1,“Clinical”,“https://maps.google.com/intl/en_us/mapfiles/ms/micons/purple-dot.png”)

versions(id, version) Example: (1,“1.9.0”)

@burke @harsha89 @cintiadr Is there anything else to add to the Add/Edit/Delete markers PR? I feel like a lot of changes to be made (like the REST API calls) depend on the PR. I could make the change in the same PR but I feel like it would be easier to review changes if I create a new PR after this one gets merged.

Those seem like reasonable suggestions.

I pulled down a local copy of your PR and tried to get it working, but couldn’t get the LDAP connection to work. I cloned the openmrs-contrib-ansible-docker-compose repo and started up a local copy of ldap-stg using docker-compose (I tweaked the docker-compose.yml to put its openldap service onto the same network as atlas. I was able to fire up Atlas via docker-compose, but I couldn’t seem to get the environment variables for the LDAP connection correct. I think the closest I got was using ldap://openldap:636 with credentials admin/admin, but it seemed to close the connection. I also tried ldap://openldap:389 but couldn’t get that to work. Port 3389 was refused. I could tell it was resolving the openldap address and reaching it and I could successfully run ldapsearch from within the openldap container for admin/admin. Then I tried manually running the atlas service container, using npm start to start the server, hoping I could debug the problem. Unfortunately, I’m not knowledgeable enough with ldap to troubleshoot.

I can try again when I get a chance… maybe coming at it fresh will help. If you can confirm the ldap URI, username, and password I should be using, it would help.

I tried it out and I’m having the same issue using docker. I kept getting ‘connection refused’ when I tried to sign in. I tried both openldap and localhospt, and ports 389 and 636.

I could quickly get atlas going using nodemon though. Here’s the steps I had to follow to get it working:

Using nodemon: