pharmaR / riskassessment

Collaborative Deployment: https://app.pharmar.org/riskassessment/ Risk Assessment Demo App: https://rinpharma.shinyapps.io/riskassessment
https://pharmar.github.io/riskassessment/
Other
101 stars 28 forks source link

Accommodate user decision upon upload. #663

Closed AARON-CLARK closed 3 months ago

AARON-CLARK commented 11 months ago

Similar to #487, users need to upload large csv's of pkg names with the final decisions already made. Specifically, if I'm a pharma org, there's a high probability that I already have a list of GXP qualified pkgs that I've developed outside of the app. This list is likely hundreds or thousands of pkgs deep. So, I propose allowing users to include an optional column to their CSV files called "decision" that aligns with the decision categories that exist in the inst/db_config.yml file. An error modal will have to appear if the values don't coincide with the expected decision values.

Also, keep in mind #662 because it would impact the work on this issue. Specifically, it would create the need for two columns: user_decision and final_decision.

A good follow up question would be that if the user has already made up their mind about this package, should we even run all the riskmetric checks on these packages? I would say "no" because the compute time needed to run riskmetric on thousands of pkgs would be insane, but we don't have a good way of re-running assessments for a package until you delete and re-upload. Perhaps we can incorporate a solution after #534.

AARON-CLARK commented 11 months ago

Update: as of 2023-11-7 dev meeting, we are also exploring the possibility of initializing a db outside of the app (#691), which would help orgs upload a bunch of pkgs at once.

jthompson-arcus commented 4 months ago

@aclark02-arcus how do you see this interacting with the rescoring/reassessing option in the metric weight tab? Should we simply make it optional on whether or not package decisions are reset/erased? We don't have any real indictors within the database itself to let the application know what process led to a decision.

aclark02-arcus commented 4 months ago

@jthompson-arcus, that's a good question. Technically, once the app is able to display the same package by date-uploaded, this question is kind of a moot point.... since packages can only be reviewed at a single point in time and re-uploading that package would initiate a new upload timestamp.

But even in that context, a package could be uploaded on the same day, so Its worthwhile to invest in a strategy. I think it would be nice to ask the user how they want to proceed and keep it simple with two options:

Replace existing decisions with csv decisions

One question is if they have a package in the csv, with no decision in the decision column, does that clear out the decision in the db?? I'd say... yes... I think.

jthompson-arcus commented 4 months ago

Your reply makes things less clear to me. So you also see this as a process to quickly assign decisions to packages currently in the database? I'm not sure if I'm on board with that because currently if a package is a duplicate, we do not refresh its data. It feels weird that a decision column would have a different interaction.

jthompson-arcus commented 4 months ago

Regardless, I was actually referring to our rescoring mechanism. More specifically these lines.

aclark02-arcus commented 4 months ago

@aclark02-arcus how do you see this interacting with the rescoring/reassessing option in the metric weight tab? Should we simply make it optional on whether or not package decisions are reset/erased?

... I was actually referring to our rescoring mechanism. More specifically these lines.

Ah, I see. You're referring to when the the re-weighting module will reset any decision previously made? Well, I don't think that is an ideal experience either. I composed a separate issue dedicated to risk re-calculation & retaining pkg decisions back in 2021 here. I think both the risk-reweighting and csv upload process should have similar options on how to handle conflicts with existing decisions.

We don't have any real indictors within the database itself to let the application know what process led to a decision.

We do. Right now, we know whether a decision was made through automation or manually via the Decision By field. That's stored in the db, no? It seems we could update the Decision By field to include the decision source & name to keep them all straight. Perhaps the value stored could be "CSV - [username]". I think it's also important to note that only admin or leads can should have the option execute a pkg decision using CSV, per existing privileges.

image

So you also see this as a process to quickly assign decisions to packages currently in the database? I'm not sure if I'm on board with that because currently if a package is a duplicate, we do not refresh its data. It feels weird that a decision column would have a different interaction.

💯! I see the dilemma. But I'd argue it's a better path forward than the current workflow since the app hasn't addressed assessment/upload timepoints #534 well yet. Let's say I have 800 pkgs in the db that we reviewed and made decisions for last year. This year, we have to review 500 of them because they have new versions available (for example). As the process stands right now, there is no way to re-assess these pkgs without (1) changing the weights or (2) manually deleting the 500 pkg, then re-uploading with a csv. I guess you could just archive the database and start over, but then we'd "lose" all the work you did on the other 300... and the history for all 800 pkgs.

I don't know... After talking this out, maybe we need to work on #534 first to do this right? lol What are your thoughts?

jthompson-arcus commented 4 months ago

I think these are all good talking points but I'm leaning towards the simplest implementation for the initial feature inclusion, which in my mind means just adding the decision if it is a new upload. I feel that if we want a process whereby a user could mass assign decisions, it would be better to create a different process than shoehorning it into the upload process.

I think for now the solution to the rescoring is to add a checkbox (defaulted to no) asking if the user wants to reset package decisions.

aclark02-arcus commented 3 months ago

closed with #777