GSOC 2019: OpenMRS Atlas 3.1

I converted your original post to a wiki post, so hopefully you’ll be able to edit it now.

2 Likes

@harsha89 Thank you! :slight_smile:

@burke Yup, I’ve updated the post. Do let me know if you’d like me to add something. I will update the Atlas 3.1 project page tomorrow. :slight_smile:

@burke @harsha89 @cintiadr

Today’s Progress:

  • Most of my time went towards figuring out how stuff actually came together in the maps section of the atlas.
  • I fixed a bug where markers provided no edit/delete options.
  • Another bug caused newly added markers be associated with a blank username.
  • I’m working towards saving newly added markers into the database.

Here’s the branch containing the new changes.

I’m a bit confused about how I should implement the add marker design. My current idea of how adding markers should work is that we click the “Add new site”, which drops a marker into the center of the map. We click the marker to find edit/delete buttons. Screenshot%20from%202019-05-27%2022-22-30

Edit brings up a little form, while delete pops up a confirmation.

Editing the form and hitting save must save the marker in the database.Screenshot%20from%202019-05-27%2022-23-24

But this procedure seems to invalidate the “Edit my site” button.

Screenshot%20from%202019-05-27%2022-26-23

I feel like I’m missing something.

How is this procedure supposed to be or how was it before?

1 Like

Look, I’m not entirely sure.

The code seems to imply that it selects some of your markers? https://github.com/openmrs/openmrs-contrib-atlas/blob/f807801e5f4c90460d4f1e22389aab7f630d421b/public/js/user.js#L14

@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?