Move 'not-patient-indicators' to the database with web interface
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)
-
reporter -
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>>
-
reporter Committed and pushed to stage without a link in admin menu! Try again! Refs
#510→ <<cset f7a3130d759b>>
-
reporter Improved grammar, added em tags. Refs
#510, will stage again.→ <<cset 6d322a626a4f>>
-
reporter Available to view at http://testing.openrem.org/admin/notpatientindicators/ if anyone wants to comment?
Is it ok to just make use of the Django backend admin interface, or should I not be so lazy and add in my own form?
@dplatten @tcdewit @LuukO comments?
-
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.
-
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.
-
reporter Thanks guys!
I'll add in the code to make it actually use those values and add some documentation next.
-
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)?
-
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?
-
fnmatch would be sufficient I think!
-
reporter First attempt to implement using patterns from the database with fnmatch(). Refs
#510→ <<cset 7b6e84c0ede0>>
-
reporter Corrected and improved get_not_pt and added a test suite Refs
#510→ <<cset 62106b181adc>>
-
reporter Doc strings for tests. Refs
#510→ <<cset 8450991d3e48>>
-
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.
-
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.
-
reporter What was I thinking!
-
reporter -
Probably. I'd tend towards normal human-readable text for anything that shows up on the web pages.
-
reporter Added in 0.7.4 no-patient patterns to Siemens CT import tests to enable testing of ref
#510with real data.→ <<cset 64de6311a011>>
-
reporter Overhaul of not-patient page. Added function to setup matching patterns to continue 0.7.4 behaviour. Refs
#510→ <<cset b72658115f2f>>
-
reporter Modified the tests to reflect the change to record patterns in lower case. Refs
#510→ <<cset 0510e8e10585>>
-
reporter ID code example had gone missing! Refs
#510→ <<cset 279bed382472>>
-
reporter Changing the layout and content of not-patient template. Refs
#510→ <<cset 313ad9c672fb>>
-
reporter Changed colours, minor text, put example and wildcards into two columns. Refs
#510→ <<cset 5be4ae09381d>>
-
reporter Took example out of panel, other minor mods. Refs
#510→ <<cset 3355ce0b14bc>>
-
reporter Resized to fit md-8. Refs
#510→ <<cset 051526fddcd5>>
-
reporter Resized to fit md-10. Refs
#510→ <<cset 3c0de1e8eda9>>
-
reporter @dplatten @LuukO @tcdewit if any of you have a chance to look at http://testing.openrem.org/admin/notpatientindicators/ I'd appreciate your view as to whether it makes sense etc.
Feel free to delete or add to the existing patterns, and to try the link at the bottom that restores the current (0.7.4) behaviour.
-
reporter Created the start of the docs for non-patient indicators. Refs
#510→ <<cset 890c9546ddff>>
-
reporter A little more on the docs. Added link from interface. Refs
#510→ <<cset e9099839fc51>>
-
reporter Attempting a more simple table. refs
#510→ <<cset 1ffdb3cc4554>>
-
reporter Attempt to get start to remain as star. Refs
#510→ <<cset 9faa9e7cc884>>
-
reporter Added explanation of the patterns. Refs
#510→ <<cset 7414d9aa5bf6>>
-
It think it is fine, especially with the updated docs that explains the patterns.
-
reporter Thanks @LuukO
-
reporter Minor change to example. Refs
#510→ <<cset adc016f8d0dc>>
-
reporter Given in - will be doing forms for not-patient indicators after all. Refs
#510→ <<cset 1c749fd8cccd>>
-
reporter Added add, modify and delete links for names. Refs
#510→ <<cset be0d5b173e4c>>
-
reporter Full set of views and templates with basic text for adding, modifying, and deleting name patterns. Needs text review. Refs
#510→ <<cset 18ea1225d590>>
-
reporter Added the current docs text to the add/change form template. Refs
#510→ <<cset 61144b4f0023>>
-
reporter Added link to replicate 0.7.4. Refs
#510→ <<cset cac3c7295a1d>>
-
@edmcdonagh, the new patient indicators page works as I would expect it to. Thanks.
-
reporter Moved most of the create/modify template to share it between name and ID. Refs
#510→ <<cset a48e0d9ce8a5>>
-
reporter Converted ID patterns to be managed in the same way as name patterns. Refs
#510→ <<cset d9e615f4f260>>
-
reporter Adding a note about not affecting existing imports. Refs
#510→ <<cset eb641e4a08dc>>
-
reporter Changed text in bottom panel. Refs
#510→ <<cset 9bbc6da09c9f>>
-
reporter It appears that this file was never committed? Refs
#510→ <<cset 913ea1d79ce8>>
-
reporter Added new config menu image, changed existing config menu topics. trying to have just one. Refs
#510→ <<cset 507c77b8d3f6>>
-
reporter Added note about pre-0.8 and config menu. Refs
#510→ <<cset 32255d07deae>>
-
reporter Trying to make width of image appropriate. Refs
#510→ <<cset 2fb40940bd80>>
-
reporter Moving the menu up the page and making it slightly bigger. Refs
#510→ <<cset cf6e9b4fd0ca>>
-
reporter Reducing the size of the config menu on delete settings, seeing if next image will slide alongside nicely. Refs
#510→ <<cset cdda644f7ec7>>
-
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>>
-
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? :) -
reporter Removed alternative admin menus, hopefully all redundant now. Formatted other uses of new config menu. Refs
#510→ <<cset 6d99b4de3d81>>
-
reporter More formatting. Refs
#510. Also removed the section about finding your IP address and changed it for using 0.0.0.0→ <<cset f5cbcdace93c>>
-
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>>
-
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...
-
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...
-
reporter Not really related to ref
#510, but it has been bothering me. @dplatten - any objection to removing chart message when not logged in?→ <<cset b7834355a268>>
-
reporter Added a panel to the home page and a singleton model for recording if not-patient indicator question should be asked. Refs
#510→ <<cset 66100c9a3682>>
-
reporter Now passing admin questions as a new dict plus a bool to determine whether to display the panel or not. Refs
#510→ <<cset 238661f86f80>>
-
reporter Fixed test for admingroup so it works when not in that group! Refs
#510→ <<cset 943a7db294d3>>
-
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>>
-
reporter - changed status to resolved
-
reporter Issue
#275was marked as a duplicate of this issue. -
reporter Removed not-patient indicators from settings, as was made redundant by ref
#510.→ <<cset 041dd159b6d2>>
- Log in to comment
Very basic initial view of not patient indicators. Refs
#510→ <<cset 2a8dde5bb3bf>>