search

Chemical-Curation / chemcurator_vuejs

Chemical registration and curation tool
https://chemreg.epa.gov
1 stars 0 forks source link

VueJS Store usage for substances and compounds #122

Open cmgrulke opened 3 years ago

cmgrulke commented 3 years ago

As a developer I want usage of the store and usage of component variables have a clear pattern and match with business logic So that the code is clean and easy to understand and bugs are less likely

Acceptance Criteria: A plan for refactoring and use of the store is outlined and documented? The code is refactored to match the plan The changes are not apparent to the user (i.e. no bugs introduced)

Test(s):

jtumkur commented 3 years ago
debboutr commented 3 years ago

@cmgrulke couple of questions:

  1. Like I said I have been working out a way to navigate to a given substance page from either the nav bar search or the Tree, with what I have so far, the tree will send the user directly to a URL with the DTXSID in it http://localhost:8080/substance/DTXSID502000000, which can just hit the django API directly. When coming from the nav bar the URL will look like so http://localhost:8080/substance?search=Solo%20substance with the search term as a query parameter, so on either the refresh will keep you right where you are at. The question here is if the search term begins with DTXSID can we just use the same API call that would go straight to django and not hit resolver? or would you rather use the search filter that reaches out to resolver? /substances?filter[search]=DTXSID602000001 vs. /substances/DTXSID602000001 the biggest difference being, we can use ?include=associatedCompound in the latter and not have to do another call to get the related compound data.

  2. In the SubstanceForm I had put a ClearForm button, would it be better to have something like a Reset Form which would in the case of being at a given substance's URL reset the data in the form to what the given substance originally had? and in the case of there not being a substance, would clear out any selected data?

cmgrulke commented 3 years ago

For 1, i am indifferent.
For 2, yes. I will create a ticket.

debboutr commented 3 years ago

okay, I'm going to set it up so that if the search string starts with DTXSID we'll just query django directly and if not, we'll use the search filter out to resolver.

Another quick Q, if you click on Substances in the Nav Bar would you like for the user to see an empty page? meaning no compound or substance data. Right now you will just be left with where you are at if you are currently on the page or if you had been there before the same data will persist. So basically, should clicking on the nav bar substances clear the page? Sorry, but I ma realizing this just now, if you have a substance loaded with a compound, then navigate to say Vocabs and then back, the substance data persists, but the compound data does not. There is also a consideration here with there then being altered data in the compound window and then a click on Nav Substances as to there being a prompt for lost data, but knowing what you'd like to have happen here would help.

cmgrulke commented 3 years ago

I would like the user to get an empty page after clicking on "substances" in the navbar.

debboutr commented 3 years ago

@cmgrulke if a user has something drawn in a given editor and were to do a search for a substance in the nav bar and it brings them to a substance that either a) has no associated compound OR b) has a compound of the opposite type, i.e ketcher/marvin would you want the data in the original editor to be removed?

cmgrulke commented 3 years ago

yes.

debboutr commented 3 years ago

here's something which I may need to solve for in order to get a PATCH in properly: if I arrive at a substance page that is intiially associated to an illDefinedCompound should I be able to then associate it with a DefinedCompound? Also the flip of this? in either of these cases the user would have to draw a known compound OR save what is drawn in order to save the substance, but when they initially flip to the different editor, it will be blank <- this is the tough part, we will need to tell them that there needs to be a compoundID in the form, until then, they shouldn't be able to save the substance, even if they have made changes to the inputs in the form itself?

debboutr commented 3 years ago

@cmgrulke in the event that a user is either A) on a know substance OR B) on a blank substance page...

if B... if the user changes the substance form inputs the "Save Substance" button you would expect to be enabled. How will this play into the QST dropdown that is selected? i.e if there have been changes to the substance form inputs and the QST type is still none, meaning no editor is in the view, then the button should be acitve and you can save the substance w/o a compound, but in the case where the QST type was Defined should the button be disabled?

I guess my question for A is much the same, it seems to me the ability for a substance to be saved in the case where one of the editors is open should depend on there being a CID in the compound input for the save to be possible.

cmgrulke commented 3 years ago

When a user changes the structure type from defined to ill-defined or vice versa, the user should be required to draw and save the compound prior to saving the substance. If the structure type is set to "none", then a save should be possible (it would set the assocatedCompound to null), but if the structure type is set to defined or one of the ill-defined and the compound id is null, the button should be disabled.

Sorry for the complex business logic around how the UI should operate here without sufficient documentation. I really should have had some wire diagramed workflows here to clarify the way that I expect users will navigate through the interface.

debboutr commented 3 years ago

cool, this helps, thanks! There is one caveat with the defined compound where, let's say there is a substance w/ no compound, then if the user is drawing and a compound is found in ID is present, in this case the substance is savable because the compound ID is populated and the associatedCompound ID is null, the difficult part is that w/ ticket 101 we have removed the compound from being loaded in the store and made possible this newly drawn compound to be saved, so even though the "Save Compound" button is enabled, the "Save Substance" button will also be enabled to PATCH in this compound ID.

I think this works with what you are saying, but if not let me know.

debboutr commented 3 years ago

@cmgrulke one thing while i am thinking of it, in the case of the Substance view having loaded a "Deprecated" compound we should probably make a ticket for how to deal with the save buttons on substance and compound for that. Probably want to either remove them from being visible OR just keep them disabled no matter what?

cmgrulke commented 3 years ago

So if I remember correctly looking at https://github.com/Chemical-Curation/chemcurator_django/issues/39, a deprecated compound when a request is made for it will automatically forward a user to the non-deprecated compound. So, if as we are drawing a inchikey for the drawn chemical matches the inchikey for a deprecated compound, does it automatically pull up its non-deprecated form? If not, we will need a ticket to resolve. Also, if a compound is the associated compound for a substance and we then deprecate that compound, the appropriate behavior has not been laid out so needs to be ticketed. I will mint the the mnecessary issues. I think the goal at this point is to never load a deprecated compound into the substance view.

debboutr commented 3 years ago

everything seems be working as expected in django, but we don't deal with a compound having a replacedBy association in vue as far as I can tell, the compound that we have in seed data that is deprecated doesn't have a replacedBy ID which is a state that I don't think we should ever find ourselves in. So, no, it doesn't pull up it's non-deprecated form. The fact that our seed data is probably a bit off is not the only reason we are getting a deprecated compound in the editor, but will need to be remedied and be a part of the the solve, hopefully seed data will get updated soon with this case. Do we need a ticket yet for seed data, forgive me if this is incorrect, I just went to the zenhub board and searched for "seed" and nothing came up

cmgrulke commented 3 years ago

So there is a seed data ticket here: https://github.com/Chemical-Curation/chemcurator_django/issues/307. We need to groom it and figure out how to get it slipped in.