legumeinfo / web-components

A collection of Web Components for interacting with and visualizing biological data.
https://legumeinfo.github.io/web-components/
Apache License 2.0
5 stars 3 forks source link

LIS Gene Search Component #20

Closed ctcncgr closed 1 year ago

ctcncgr commented 1 year ago

Now that we can perform simple queries against terms, we can expand the search into more complex examples.

The first of these I think should be a lis-gene-search style component that uses a more complex query against the IntermineAPI.getGenes method as opposed to IntermineAPI.geneSearch.

Here is the extension of this.

https://github.com/legumeinfo/graphql-server/blob/main/src/data-sources/intermine/intermine.api.js

Files that @sammyjava thinks need to be edited for the required search form and results list (partial list):

And new files that I think are needed:

sammyjava commented 1 year ago

@alancleary @adf-ncgr This appears to be assigned to @ctcncgr while I seem to be tasked with building the gene search component. There's a lot of files that need to be changed since the specified search results are in a vertical layout, etc. Does this assignment transition to me? I've gotten started on a new local branch but haven't done a lot of work on it yet. I'm guessing this old assignment is superseded by the more recent directive.

Part of this is the urgency to get this done. I have availability. But I don't have the chops on these various pieces, like LisPaginatedSearchMixin which could slow me down a bit. Or maybe not. I do plan to make a list of files I need to change to be sure I don't overdo it.

I'll hold off on coding any more until this assignment is resolved.

adf-ncgr commented 1 year ago

yes, I think this old issue has effectively become what you are working on. We can reassign it to you or just close it and make a new one that more accurately reflects the new spec; whichever you prefer is fine with me.

sammyjava commented 1 year ago

No, this issue is fine, I needed place to jot down files that I'm changing, creating, for @alancleary 's and @ctcncgr 's cross-check, this is a good place.

alancleary commented 1 year ago

@sammyjava I have no reservations about updating dev/lis-gene-search-element.html and src/lis-gene-search-element.ts. I'm wondering, though, why you want to use src/core/lis-vertical-results-element.ts instead of the existing simple table element. Moreover, why not just use the existing paginated search mixin? That takes care of a lot of the boilerplate and will get you into production faster.

sammyjava commented 1 year ago

@sammyjava I have no reservations about updating dev/lis-gene-search-element.html and src/lis-gene-search-element.ts. I'm wondering, though, why you want to use src/core/lis-vertical-results-element.ts instead of the existing simple table element. Moreover, why not just use the existing paginated search mixin? That takes care of a lot of the boilerplate and will get you into production faster.

We don't want tabular results. Those tables appear to be written into those files. The results are a vertical stack of full-width records, as per the spec here: https://github.com/legumeinfo/component-search-ui-srs/blob/main/gene-search/README.md

But that's why I'm listing files before I do the work, because I may misunderstand how to change the layout of the search results.

The search form itself is in src/lis-gene-search-element.ts so that's no big deal.

sammyjava commented 1 year ago

@alancleary also the result items are munged together; each result display is:

gene.identifier (gene.name) gene.organism.genus gene.organism.species gene.strain.identifier gene.description location: gene.locations[0].chromosome.identifier:gene.locations[0].start-gene.locations[0].end (gene.locations[0].strand) gene family: gene.geneFamillyAssignments[0].geneFamily.identifier

which looks like this in the React prototype:

<div key={index}>
  <div>
    <b>{ gene.identifier }</b> ({ gene.name }) <span className="uk-text-italic">{ gene.organism.genus } { gene.organism.species }</span> { gene.strain.identifier }
  </div>
  <div className="uk-text-italic">
    { gene.description }
  </div>
  {gene.locations[0] && gene.locations[0].chromosome && (
      <div>
        <b>location:</b> { gene.locations[0].chromosome.identifier }:{gene.locations[0].start}-{gene.locations[0].end} ({gene.locations[0].strand})
      </div>
  )}
  {gene.geneFamilyAssignments[0] && (
      <div>
        <b>gene family:</b> { gene.geneFamilyAssignments[0].geneFamily.identifier }
      </div>
  )}
  <hr className="uk-divider-icon"/>
</div>
alancleary commented 1 year ago

We don't want tabular results. Those tables appear to be written into those files. The results are a vertical stack of full-width records, as per the spec here: https://github.com/legumeinfo/component-search-ui-srs/blob/main/gene-search/README.md

Configuring the table is part of implementing a class that uses the paginated search mixin. The table itself is actually an instance of the simple table element, which renders its cell content as HTML. So you could configure the table to not have a header and put each row's content into a single cell with whatever HTML structure you want, such as the example you just provided.

This still may not be "ideal" but I think it will be "good enough" and we can update the mixin later to be more flexible in this regard. The main thing is all the boilerplate you're going to get from the mixin. Use cases like this are the reason it exists!

sammyjava commented 1 year ago

I'm talking about lis-simple-table-element.ts which is inappropriate for the gene search results because it's a table. It seems better to create a new element with divs since it isn't a table, leaving the table for other places where a table is desired. I don't want to use a table for a vertical display. It's bad HTML practice, which I'll not engage in. I'm not going for "good enough" here, I'm going for "will last forever because it's done right."

sammyjava commented 1 year ago

@alancleary the query returns an array of Gene, with its object attributes as well as string attributes. How do I break those objects out in lis-gene-search-element.ts? I'm getting parsing errors on the two ways of doing it that I can think of:

    constructor() {
        super();
        // configure query string parameters
        this.requiredQueryStringParams = ['query'];
        // configure results table
        this.resultAttributes = [
            'name',
            'identifier',
            'description',
            'organism',
        ];
        this.tableHeader = {
            name: 'Name',
            identifier: 'Identifier',
            description: 'Description',
            organism['genus']: 'Genus',  <== PARSE ERROR
            organism.species: 'Species', <== PARSE ERROR
        };
    }

I can get this, which isn't useful (still using the table output):

Name Identifier Description Organism
Ae04g30110 aesev.CIAT22838.gnm1.ann1.Ae04g30110 protein disulfide isomerase-like protein; IPR005746 (Thioredoxin), IPR005792 (Protein disulphide isomerase), IPR012336 (Thioredoxin-like fold); GO:0005783 (endoplasmic reticulum), GO:0006662 (glycerol ether metabolic process), GO:0015035 (protein disulfide oxidoreductase activity), GO:0016853 (isomerase activity), GO:0045454 (cell redox homeostasis) [object Object]
alancleary commented 1 year ago

The LisSimpleTableElement uses the resultsAttributes property to key into the tableHeader object and each results object. In other words, the attributes of these objects should match the the resultAttributes array, which may require flattening the GraphQL results before giving them to the component.

Regarding:

It's bad HTML practice, which I'll not engage in. I'm not going for "good enough" here, I'm going for "will last forever because it's done right."

It sounds like updating the paginated search mixin to accommodate this use case will be the solution then, since implementing a whole new component/mixin will require rewriting a bunch of general purpose code the mixin already provides.

sammyjava commented 1 year ago

OMG I totally forgot about the flattening trick, which I did in the QTL search with the underscore switcheroo! Thanks!

sammyjava commented 1 year ago

It sounds like updating the paginated search mixin to accommodate this use case will be the solution then, since implementing a whole new component/mixin will require rewriting a bunch of general purpose code the mixin already provides.

And this is just a branch so we'll have a chance to review if it makes sense. So I'll update, not create new. It could be we never go with tabular results, anyway, to provide a consistent look and feel.

alancleary commented 1 year ago

And this is just a branch so we'll have a chance to review if it makes sense. So I'll update, not create new. It could be we never go with tabular results, anyway, to provide a consistent look and feel.

I agree a consistent look and feel is important. However, limiting all search elements to displaying their results in a table is probably too restrictive. And probably less than optimal too; I used a table when coding up a prototype because "why not?" and it has perpetuated into other elements!

sammyjava commented 1 year ago

OK a related problem with flattening: it appears it only works one level deep, and not on array elements, but we can have arbitrarily deep levels to objects within objects, which can be arrays.

Here's what happens when I flatten a gene search result. organism_genus, organism_strain, and strain_identifier are good to go, but geneFamilyAssignments is an array that doesn't get its elements flattened, and locations is an array that doesn't get its elements flattened. In the React demo I work directly with the objects and just pull geneFamilyAssignments[0] and locations[0], but that's not amenable here. And also not general purpose. (I've chosen to display only the first gene family assignment and location, which is what we want in this specific case, but that's not generally true.)

What I'd like is to somehow get geneFamilyAssignments_0_geneFamily_identifier and locations_0_chromosome_identifier, locations_0_start, locations_0_end, locations_0_strand out of the flatten function.

  {
    "name": "Ae04g30110",
    "identifier": "aesev.CIAT22838.gnm1.ann1.Ae04g30110",
    "description": "protein disulfide isomerase-like protein; IPR005746 (Thioredoxin), IPR005792 (Protein disulphide isomerase), IPR012336 (Thioredoxin-like fold); GO:0005783 (endoplasmic reticulum), GO:0006662 (glycerol ether metabolic process), GO:0015035 (protein disulfide oxidoreductase activity), GO:0016853 (isomerase activity), GO:0045454 (cell redox homeostasis)",
    "organism_genus": "Aeschynomene",
    "organism_species": "evenia",
    "strain_identifier": "CIAT22838",
    "geneFamilyAssignments": [
      {
        "geneFamily": {
          "identifier": "legfed_v1_0.L_3QTWRW"
        }
      }
    ],
    "locations": [
      {
        "chromosome": {
          "identifier": "aesev.CIAT22838.gnm1.Ae04"
        },
        "start": 29908706,
        "end": 29912088,
        "strand": "1"
      }
    ]
  },

We define flatten in graphql.js as so:

// Flatten GraphQL results that contain objects
const flatten = (obj, out={}, prefixes=[]) => {
    if (obj != null) {
        Object.keys(obj).forEach(key => {
            const key_prefixes = [...prefixes, key];
            if (typeof obj[key] == 'object' && !Array.isArray(obj[key])) {
                out = flatten(obj[key], out, key_prefixes);
            } else {
                const prefix_key = key_prefixes.join('_');
                out[prefix_key] = obj[key];
            }
        });
    }
    return out;
};

Can that be tweaked to do what I want? I'm a bit reluctant to dive into a flattening method project. :)

sammyjava commented 1 year ago

Here's another question. I have absolutely no idea how to populate the selectors dynamically in this framework. I need to do the following, from React:

`              <select class="uk-select uk-form-small" name="genus" value={selectedGenus}
                      onChange={e => handleGenusSelection(e)}>
                <option value="">-- any --</option>
                {genusList.map((genus, index) => (
                <option key={index} value={genus}>{genus}</option>
                ))}
              </select>

the value attribute sets the selected genus value (which is needed if it's passed in from a query string); perhaps I need an if-then under options. The genusList is from a static query, but, of course speciesList and strainList will be dynamically queried based on genus and species. I know you mentioned that the paging stuff dynamically updates, but I'd appreciate some pointers into how to get started on this, since I've not done anything similar to date.

(In case it's not obvious, handleGenusSelection sets the speciesList state property.)

alancleary commented 1 year ago

Can that be tweaked to do what I want? I'm a bit reluctant to dive into a flattening method project. :)

Easy fix. Just need to call flatten on each element in the array instead of doing nothing:

// Flatten GraphQL results that contain objects
const flatten = (obj, out={}, prefixes=[]) => {
    if (obj != null && typeof obj == 'object' && !Array.isArray(obj)) {
        Object.keys(obj).forEach(key => {
            const key_prefixes = [...prefixes, key];
            if (typeof obj[key] == 'object' && !Array.isArray(obj[key])) {
                out = flatten(obj[key], out, key_prefixes);
            } else {
                const prefix_key = key_prefixes.join('_');
                if (Array.isArray(obj[key])) {
                    out[prefix_key] = obj[key].map((value) => flatten(value));
                } else {
                    out[prefix_key] = obj[key];
                }
            }
        });
    }
    return out;
};
sammyjava commented 1 year ago

Easy fix. Just need to call flatten on each element in the array instead of doing nothing:

OK, but I can't access locations[0].chromosome_identifier, either. I think the arrays have to be flattened out as well. I'm looking for locations_0_chromosome_identifier as a top-level attribute so I can use it. Unless there is some way to get an array element that I've not tried. Same with geneFamilyAssignments, of course.

{
  "name": "Ae04g30110",
  "identifier": "aesev.CIAT22838.gnm1.ann1.Ae04g30110",
  "description": "protein disulfide isomerase-like protein; IPR005746 (Thioredoxin), IPR005792 (Protein disulphide isomerase), IPR012336 (Thioredoxin-like fold); GO:0005783 (endoplasmic reticulum), GO:0006662 (glycerol ether metabolic process), GO:0015035 (protein disulfide oxidoreductase activity), GO:0016853 (isomerase activity), GO:0045454 (cell redox homeostasis)",
  "organism_genus": "Aeschynomene",
  "organism_species": "evenia",
  "strain_identifier": "CIAT22838",
  "geneFamilyAssignments": [
    {
      "geneFamily_identifier": "legfed_v1_0.L_3QTWRW"
    }
  ],
  "locations": [
    {
      "chromosome_identifier": "aesev.CIAT22838.gnm1.Ae04",
      "start": 29908706,
      "end": 29912088,
      "strand": "1"
    }
  ]
}
alancleary commented 1 year ago

I have absolutely no idea how to populate the selectors dynamically in this framework.

Check out the Lit documentation on reactive properties. I think the thing to do would be get all the form data into the component via one or more @property attributes and then maintain each form element's state in a @state attribute. Use the states to give the form elements their values in the component's template. This will cause Lit to automatically redraw the template when one of their values changes.

To dynamically change these element's states, you'll want to react to interactions with specific form elements using Lit's support for event listeners. In this case you want to listen for change events instead of the click event Lit uses as an example. I'm not actually sure if this will fire if you change a @state attribute's value. If it doesn't then you'll have to do something like using the hasChanged lifecycle hook to propagate the changes across form elements.

alancleary commented 1 year ago

OK, but I can't access locations[0].chromosome_identifier, either. I think the arrays have to be flattened out as well. I'm looking for locations_0_chromosome_identifier as a top-level attribute so I can use it. Unless there is some way to get an array element that I've not tried. Same with geneFamilyAssignments, of course.

This sounds like too specific of a use case to include in the flatten function since there may be scenarios where the array should be left intact. I suggest adding a small function to your GraphQL-to-Component shim that you can use to accomplish this for both the location and gene family properties.

sammyjava commented 1 year ago

This sounds like too specific of a use case to include in the flatten function since there may be scenarios where the array should be left intact. I suggest adding a small function to your GraphQL-to-Component shim that you can use to accomplish this for both the location and gene family properties.

Arrays are extremely common objects from GraphQL queries: they are what you get from any [Object] Type. It is most definitely not a specific use case. Every InterMine collection leads to an array in the GraphQL type definition. We need a general solution, not a specific one.

Granted, in general we'll want all of the array elements, not just the first one. And that would be great if we can implement a way to get all of them in an array in lis-gene-search-element.ts, etc. Flattening is a bad solution since our data are far from flat. (For example, we should be showing multiple gene families to include the Phytozome ones and newer versions of the LIS ones even on the gene search. It's a loss of information to only pull the first.)

sammyjava commented 1 year ago

To reiterate (and put the task into a concrete form), we need to be able to handle an object like this, in general, with potentially even deeper objects and arrays. I don't believe that flattening is the solution. I think we need to be able to handle JSON objects in the renderer, just as React does.

{
  "name": "Ae04g30110",
  "identifier": "aesev.CIAT22838.gnm1.ann1.Ae04g30110",
  "description": "protein disulfide isomerase-like protein; IPR005746 (Thioredoxin), ..., GO:0045454 (cell redox homeostasis)",
  "organism": {
    "genus": "Aeschynomene",
    "species": "evenia"
  },
  "strain": {
    "identifier": "CIAT22838"
  },
  "geneFamilyAssignments": [
    {
      "geneFamily": {
        "identifier": "legfed_v1_0.L_3QTWRW"
      },
    },
    {
      "geneFamily": {
        "identifier": "phytozome_foobar_10101"
      },
    },
    {
      "geneFamily": {
        "identifier": "yet_another_gene_family_we_care_about",
      }
    }
  ],
  "locations": [
    {
      "chromosome": {
        "identifier": "aesev.CIAT22838.gnm1.Ae04"
      },
      "start": 29908706,
      "end": 29912088,
      "strand": "1"
    },
    {
      "chromosome": {
        "identifier": "we_have_single_locations_in_LIS_but_its_a_worthy_example"
      },
      "start": 12345678,
      "end": 12356789,
      "strand": "-1"
    }
  ]
}
alancleary commented 1 year ago

To reiterate (and put the task into a concrete form), we need to be able to handle an object like this, in general, with potentially even deeper objects and arrays. I don't believe that flattening is the solution. I think we need to be able to handle JSON objects in the renderer, just as React does.

That's fine. Like I said before, the flattening is a limitation of the simple table component (it's simple!). It sounds like you're going to replace that anyway so there's no reason you have to keep the data flattening requirement.

I'll go ahead and make a branch that encapsulates the table portion of the paginated search component's template in a method so it will still be used as the default way to display results but you can easily override it with your own template in the implementing class, in which case it'll be up to you to parse and display the data however you see fit. I'll do that now.

alancleary commented 1 year ago

The results template encapsulation has been implemented in the paginated-search-results-template branch of the repo. There's an example of how to do a template override in a docstring comment.

Note that class properties related to the simple table have been left in place since it's still the default. This is pretty much just for backwards compatibility.

alancleary commented 1 year ago

@sammyjava I've pushed my web component interpretation of your React prototype to the gene-search-update branch. I still need to implement querystring parameters, but everything else seems to be to spec.

However, when I use any inputs besides Identifier and description, the chromosome entry in the results is null. I'm not sure if this behavior is correct or not so I'm not handling it in the component or the example code. If it is correct, then I think it should be handled before the component since our components are supposed to be dumb, i.e. the components say exactly what data they're expecting and that's what they should be given.

Edit: I forgot to mention that I somewhat flattened the data the component consumes as compared to what comes from GraphQL since most of the nesting was gratuitous as far as the component is concerned. There's still some structure, though. And more can added/unflattened as the need arises.

sammyjava commented 1 year ago

There are many gene records in MiniMine that lack chromosome locations because they are imported from gene families and do not get located on genomes. You should switch to GraphQL querying LegumeMine to avoid that situation, which I think is likely what you're hitting. That's what I did for my demo.

sammyjava commented 1 year ago

@sammyjava I've pushed my web component interpretation of your React prototype to the gene-search-update branch. I still need to implement querystring parameters, but everything else seems to be to spec.

However, when I use any inputs besides Identifier and description, the chromosome entry in the results is null. I'm not sure if this behavior is correct or not so I'm not handling it in the component or the example code. If it is correct, then I think it should be handled before the component since our components are supposed to be dumb, i.e. the components say exactly what data they're expecting and that's what they should be given.

Edit: I forgot to mention that I somewhat flattened the data the component consumes as compared to what comes from GraphQL since most of the nesting was gratuitous as far as the component is concerned. There's still some structure, though. And more can added/unflattened as the need arises.

Is there a reason to not use the GraphQL provided JSON object, though? Some, not all, flattening sounds problematic. We really have very non-flat objects coming from GraphQL and it seems that we should be able to use that. Then there is no "as need arises". You just deal with the GraphQL object.

alancleary commented 1 year ago

Is there a reason to not use the GraphQL provided JSON object, though? Some, not all, flattening sounds problematic. We really have very non-flat objects coming from GraphQL and it seems that we should be able to use that. Then there is no "as need arises". You just deal with the GraphQL object.

Here's 3 reason ordered from most principal principle to most pragmatic:

  1. Loose coupling - This enhances component portabliity while making them more resilient to changes in the GraphQL API and/or query structure (there's more than one query that will get you the same data!).
  2. Data model - Odds are the data model that's best for the component is not the structure of the GraphQL object, so you're going to have to "deal with the GraphQL object" somewhere in the dataflow.
  3. Users - As we've already seen, users (i.e. developers) are going to be tinkering with the data anyway before giving it to the component (adding links, putting additional data into certain fields, adding styling, etc), so putting the burden of restructuring on them is reasonable.

In this case, the partial flattening I mentioned is pretty benign so it's hard to really see the benefit. The more compelling example is the GeneSearchFormData type used by the search component. This type is a significant restructuring of the GraphQL query that provides its data (it actually adds depth instead of flattening!), but the effect is that the dynamic, interconnected select elements were very easy to implement.

Edit: Apparently I can't spell :laughing:

sammyjava commented 1 year ago

Taking a look at gene-search-update. getGeneFormData is currently only returning ten organisms and therefore only five genera to the selector. This is because the size parameter is not supplied to override the default of 10. The query used in the react demo was on organisms(size: $size) with the variables object in the JSON holding size: 100. I think you said yesterday that you were fixing this.

But at a higher level, I don't think that default of 10 records should be in graphql-server. It shouldn't assert what the default page size is. I think the default should be it returns all records (which is the mine default). The consuming app should decide what page size to use. It's a bit clunky to have to supply a large size to get a full result set back.

Otherwise stuff looks good. I'll hold off on doing anything until the search form selectors are fully populated. Presumably what I need to work on is the linkouts, right? Since you've got the search implemented. Or maybe I'm totally off the hook on this one.

sammyjava commented 1 year ago

Oh, there is one issue that I had thrown on the back burner: genes that are on supercontigs, not chromosomes. That's currently throwing an error here because chromosome is null. I'll get that working in the React demo and I'll update this app later on. We'll have to support either chromosome or supercontig being populated in the returned object. Doesn't seem too hard.

alancleary commented 1 year ago

Taking a look at gene-search-update. getGeneFormData is currently only returning ten organisms and therefore only five genera to the selector. This is because the size parameter is not supplied to override the default of 10. The query used in the react demo was on organisms(size: $size) with the variables object in the JSON holding size: 100. I think you said yesterday that you were fixing this.

But at a higher level, I don't think that default of 10 records should be in graphql-server. It shouldn't assert what the default page size is. I think the default should be it returns all records (which is the mine default). The consuming app should decide what page size to use. It's a bit clunky to have to supply a large size to get a full result set back.

Thanks for bringing this up. I had to remove the size: 100 argument because those data are now being fetched via a nested query and currently our API only supports page sizes at the entrypoint level, not in nested structures. I agree that removing the default page size on the server side is a good idea. I think it was hard-coded when we were first starting out purely for convenience.

Otherwise stuff looks good. I'll hold off on doing anything until the search form selectors are fully populated. Presumably what I need to work on is the linkouts, right? Since you've got the search implemented. Or maybe I'm totally off the hook on this one.

It sounds like nothing needs to change in the component for the selectors to be fully populated, i.e. removing the default page size on the server should take care of it. Linkouts won't require any changes to the component either. The component renders all data its given as HTML so the linkout functionality can be implemented by including hyperlinks that trigger linkouts in the data given to the component. The gene search with linkouts example shows how to do this - although I completely forgot about that example until now so it's currently broken in the gene-search-update branch... I'll fix that now. Also, the linkuot service needs to be added to the GraphQL API at some point, ideally before using linkouts in production.

Not sure if you saw I pushed some changes to the gene-search-update branch about 30 minutes ago. These changes implement querystring parameter support. The one breaking change that I'll note here is that the requiredQueryStringParams property of the paginated search mixin is now a 2D array so more complex sets of parameters can be expressed, e.g. in the gene search component genus, genus+species, and genus+species+strain querystring parameters will all trigger a search when the component is first loaded, but species by itself will not. The other search components have been updated to use this new syntax.

Now I just need to fix the next page bug.

alancleary commented 1 year ago

Oh, there is one issue that I had thrown on the back burner: genes that are on supercontigs, not chromosomes. That's currently throwing an error here because chromosome is null. I'll get that working in the React demo and I'll update this app later on. We'll have to support either chromosome or supercontig being populated in the returned object. Doesn't seem too hard.

Unless chromosomes and supercontigs are going to be treated differently by the gene search component, for now I think we should just shove the supercontig into the chromsome field of the data before giving it to the component.

sammyjava commented 1 year ago

Well, we've made a clear distinction between chromosomes and supercontigs in the mines and the data store. I'd like to work with them separately. I'm avoiding any "for now" activities - this is going to be put into production. It shouldn't be too hard to deal with.

This is what I did in the React demo to handle it; I also do not throw errors on the graphql side if getChromosome or getSupercontig do not find an object for a given identifier, since we don't know which it is until we query and find out.

                          {gene.locations[0] && gene.locations[0].chromosome && (
                              <div>
                                <b>chromosome location:</b> { gene.locations[0].chromosome.identifier }:{gene.locations[0].start}-{gene.locations[0].end} ({gene.locations[0].strand})
                              </div>
                          )}
                          {gene.locations[0] && gene.locations[0].supercontig && (
                              <div>
                                <b>supercontig location:</b> { gene.locations[0].supercontig.identifier }:{gene.locations[0].start}-{gene.locations[0].end} ({gene.locations[0].strand})
                              </div>
                          )}
alancleary commented 1 year ago

Well, we've made a clear distinction between chromosomes and supercontigs in the mines and the data store. I'd like to work with them separately. I'm avoiding any "for now" activities - this is going to be put into production. It shouldn't be too hard to deal with.

OK, we'll distinguish them. I'm working on the example code now so I'll make this change as well.

sammyjava commented 1 year ago

Is https://github.com/legumeinfo/web-components/blob/gene-search-update/dev/examples/lis-gene-search-with-linkouts.html supposed to work? It's returning zero linkouts. image

sammyjava commented 1 year ago

Is https://github.com/legumeinfo/web-components/blob/gene-search-update/dev/examples/lis-gene-search-with-linkouts.html supposed to work? It's returning zero linkouts.

Never mind, I fixed it in e56f8e00db5d793ecbd6349c4557f7f8c29e63fe by updating the URL. I also removed it as a parameter to geneLinkouts since it makes no sense to me to have it be sent as a parameter, which strikes me as just asking for bugs. (There is only one linkout service for LIS applications, even if the idea of having multiple may be aesthetically pleasing.)

So this works, but I have a task to change it to being served by GraphQL. Which I'm not sure I agree with doing since it's just extra work for no clear reason to me. It's a microservice, sent a request to it.

sammyjava commented 1 year ago

Let's see, I guess it's time to switch to grabbing linkouts from GraphQL now that graphql-server supports that. I'd like you to do that here @alancleary and then I'll handle loading the component onto the Jekyll site.

I think we have some pushes to main before this whole thing goes into production:

alancleary commented 1 year ago

We still need to resolve component-search-ui-srs issues #5. I also still need to implement support for adding links to chromosome/supercontig locations. I can implement both quickly once we have consensus on the data loading UI element.

Regarding putting this into production, I still want to get automated builds triggered by tagged releases in place. This might not get done before I'm on PTO next week. The short-term alternative would be to tag a release and manually add the bundled and versioned production file to the release.

sammyjava commented 1 year ago

I'm not sure there's a huge hurry on getting this into production. We should probably have folks hit an "alpha" (betas?) quite a bit first, anyway.

alancleary commented 1 year ago

Done in PR #115.