1. OpenREM Avatar OpenREM
  2. OpenREM
  3. OpenREM
  4. Issues

Move 'not-patient-indicators' to the database with web interface

Issue #510 resolved
Ed McDonagh created an issue

Not patient indicators are currently hard-coded. Commits related to issue #503 have moved this to settings, it needs to be moved to the database with a suitable web interface for configuring it.

Comments (68)

  2. Ed McDonagh reporter

    Completed the view of indicators page, linked to django admin interface to edit them. Removed all other models from admin interface for now - need to make sure this is ok. Haven't linked db fields into import checks yet. Refs #510

    → <<cset 109f34034a67>>

  6. David Platten

    It looks good to me. I don't mind that you're using the backend interface, providing that the documentation about how to change these settings reflects this.

  7. Luuk

    I just had a look and agree with David. Tthe Django interface is clear enough to keep it that way. And of course it is prettier if you add your own form, but I don't think it is a necessity.

  8. Ed McDonagh reporter

    Thanks guys!

    I'll add in the code to make it actually use those values and add some documentation next.

  9. Tim de Wit

    I agree as well... however, wouldn't it be better to use explicit regexp or wildcards for matching? That might solve the problem of test studies matching a real patient (at least there's more control for the admin-user on the matching part)?

  10. Ed McDonagh reporter

    Good point @tcdewit. Allowing regular expressions scares me a little - for trying to document how to use it if for no other reason!

    Maybe using fnmatch would be a good idea though - that would allow for wildcard matching that most people are familiar with.

    Thoughts?

  15. Ed McDonagh reporter

    I've ditched the XML style output for this - @dplatten, @luuko, @tcdewit do you think anyone will care? I'm not sure why I was keen on it in the first place now!

    You can see how I've implemented it and what it looks like from the test functions

    Comments welcome. I'll do the docs when I next get to do some coding.

  16. David Platten

    I didn't like the XML style output, as I thought it looked like there was a glitch in the site code. I think the new style will be an improvement.

  19. David Platten

    Probably. I'd tend towards normal human-readable text for anything that shows up on the web pages.

  54. Ed McDonagh reporter

    Added a clearfix that probably won't work. Added brief instructions on how to add/modify/delete patterns. Changed the case of Name. Refs #510

    → <<cset b6a9485bbd49>>

  55. Tim de Wit

    Looks great! For testing replicating behaviour of release 0.7.4 administrator rights are required but I trust you implemented it correctly.
    Is that what we want though? "Opt-in" instead of "opt-out" by default? I could imagine some people might overlook this during an upgrade so perhaps behavior should stay the same by default and configurable for those who took the effort to actually read the changelog? :)

  58. Ed McDonagh reporter

    Now tests for admin group in more places and removes links if not admin. Adds messages to confirm success and error message if user still manages to craft correct URL. Refs #510, credit to @tcdewit for reminding me to check!

    → <<cset a46d255a3d93>>

  59. Ed McDonagh reporter

    @tcdewit thanks for prompting me to look at what it looks like without admin rights, and apologies for not giving you any! Wasn't my intention, and fixed now.

    I have added a username of demo demotesting too to enable us to see whait it looks like as view only.

    I think I'd rather have opt-out by default, but I think that would mean adding in data migrations which I had shied away from so as to not make the upgrade any more difficult, but now you've made me look at the instructions it doesn't look so hard...

  60. Ed McDonagh reporter

    Looks like a data migration would turn a simple migration into a complex one with scary merges because we can't know what migrations precede it, and it would depend on what I'm hoping will be an auto migration.

    I have an idea though for some messages to pop up when an admin logs in that can be implemented or cleared as the admin wishes...

  65. Ed McDonagh reporter

    Prompt to fix not-patient identifiers on home page now working. Not particularly generic, but could be extended for other questions. Refs #510

    → <<cset f3917074cbc7>>

  69. Log in to comment
Assignee
–
Type
enhancement
Priority
minor
Status
resolved
Component
Import: All
Milestone
0.8.0
Votes
0
Watchers
1
Jira Software: the preferred issue tracker for Bitbucket. Join the team!