Health-Informatics-UoN / Carrot-Mapper

Convenient And Reusable Rapid Omop Transformer.
https://carrot.ac.uk
MIT License
14 stars 3 forks source link

Automatic Rules Refresh on the Mapping Rules page #617

Closed PhilAppleby closed 2 months ago

PhilAppleby commented 8 months ago

Rules should be refreshed automatically on Mapping Rules page load.

Alternatively a warning should be displayed to alert the user of the need to refresh

spco commented 8 months ago

Refresh is only required once, after all Person ID and Date Event columns are set in all the tables. Is a warning to that effect what's required?

Could combine it with automated refresh, e.g. if the rules have never been generated, attempt to do so on page load (with a placeholder message to explain the delay), and if the Person IDs and Date Events haven't been set then present a message to that effect?

erummas commented 8 months ago

Hi Sam, There is a possibility that not all tables require mapping at that time and some may get mapped later. Delaying rule generation may not be the best idea. I think a warning message with an option just as the person goes to the 'rules page' may work stating that 'if use has added any new tables since the last mapping , rules need to be refreshed for generate new mappings'. Another way could be that every time a table is edited it goes to the 'Update Table' page with a button update "tablename.csv" so we refresh rules on that button click.

spco commented 8 months ago

Remember that rules that are added manually after initial import do not require "Refresh rules" to be pressed - their rules are generated at the moment the Concept is added. We need only run "Refresh Rules" once, after all the Person ID and Date Event columns have been set. I would assume that that information can be immediately set in all cases, as these need to have been decided before upload to ensure the dataset is in the right form.

So the workflow is

While adding the flexibility you suggest above might be nice, I think it would be a lot of work to support a workflow that we could reasonably avoid with minimal inconvenience to the user. They just need to have added Person ID and Date Event to all the tables before refreshing - we could add that logic into the warnings etc to ensure the refresh only ever runs once, at the point when it's appropriate.

erummas commented 8 months ago

Got it, usually in cases of datasets when we have a lot of tables, it is hard to keep track of what tables we have mapped and completed mapping for and thus usually add these details only when we decide to map that table. May be we could think of another ways to cater this problem. I think an error message on the 'Go to rules' button would suffice to let the user know that they need to select IDs and Event dates for all tables to be able to generate the first mapping. So once all that information is populated the button lets us navigate to the rules page.

spco commented 8 months ago

Thanks @erummas . I think that touches on another related topic, which is how we help users know what's been mapped and (crucially) what hasn't yet. So hopefully if we improve that then the workflow you highlight won't be needed.

spco commented 8 months ago

After discussion, the proposal is to move mapping rule generation for reused and vocab-mapped concepts to the upload stage. This would remove the need for a Refresh Rules button at all. Depending on the planned work, an interim measure implementing a warning as above might be useful.

AndyRae commented 2 months ago

Our rules refresh has fundamentally changed since this discussion (doesn't exist anymore really), so closing this.