informatics-isi-edu / ermrestjs

ERMrest client library in JavaScript
Apache License 2.0
4 stars 3 forks source link

Changes related to sourcekey and visible columns #992

Closed RFSH closed 1 year ago

RFSH commented 1 year ago

In this PR,

In #909 we talked about allowing source definitions to refer to other definitions like this

"tag:isrd.isi.edu,2019:source-definitions": {
  "sources": {
    "S_core_fact": {
      "source": [{"inbound": ["CFDE", "core_fact_id_namespace_fkey"]}, "nid"]
    },
    "S_taxonomy": {
      "markdown_name": "Taxonomy",
      "source": [
        {"sourcekey": "S_core_fact"},
        {"inbound": ["CFDE", "core_fact_ncbi_taxonomy_core_fact_fkey"]},
        {"outbound": ["CFDE", "core_fact_ncbi_taxonomy_ncbi_taxon_fkey"]},
        "nid"
      ]
    },
    "S_taxonomy_array": {
      "sourcekey": "S_taxonomy",
      "aggregate": "array_d"
    }
  }
}

What I pushed is just a simple syntactic sugar to avoid copy/paste, and so when this source is used in a visible-column, there is not signal that it's using a different sourcekey. So It cannot properly do the path sharing. But if we want this to also properly handle path sharing, we would need to think more about it and decide how it should be done.

RFSH commented 1 year ago

After discussing these changes, we decided to revert the extra changes I included in this PR.

While the change related to column heuristics is something we should do, we have to make sure we're doing it consistently for row-name as well. If possible, we would like to keep these two as close as possible.

And the change to allow sourcekey inside source-definitions requires more discussion.