diff --git a/src/test/test_flybase_combo_pubs.py b/src/test/test_flybase_combo_pubs.py index 55ab688..9f1d2a2 100644 --- a/src/test/test_flybase_combo_pubs.py +++ b/src/test/test_flybase_combo_pubs.py @@ -113,8 +113,21 @@ def test_fbrf_is_a_visible_column(self): # fbrf must be a normal displayed column, not the (hidden) identity. result = get_flybase_combo_pubs(KNOWN_COMBO_ID, return_dataframe=False, limit=3) fbrf = result["headers"]["fbrf"] - assert fbrf["type"] == "text" + assert fbrf["type"] == "markdown" assert fbrf["order"] >= 0 # FBco0000052 has publications, so an empty result is a defect. assert result["rows"], "KNOWN_COMBO_ID should have publications" - assert result["rows"][0]["id"] == result["rows"][0]["fbrf"] + # the hidden identity stays the bare FBrf; the visible column links it + row = result["rows"][0] + assert row["fbrf"] == f"[{row['id']}](https://flybase.org/reports/{row['id']})" + + @pytest.mark.integration + def test_identifier_columns_link_out(self): + result = get_flybase_combo_pubs(KNOWN_COMBO_ID, return_dataframe=False, limit=5) + for col in ("fbrf", "doi", "pmid", "pmcid"): + assert result["headers"][col]["type"] == "markdown", col + # title and citation are free text and must not be linkified + assert result["headers"]["title"]["type"] == "text" + assert result["headers"]["miniref"]["type"] == "text" + dois = [r["doi"] for r in result["rows"] if r["doi"]] + assert any(d.startswith("[") and "](https://doi.org/" in d for d in dois), dois diff --git a/src/test/test_flybase_stocks.py b/src/test/test_flybase_stocks.py index ae7e6d0..47979d4 100644 --- a/src/test/test_flybase_stocks.py +++ b/src/test/test_flybase_stocks.py @@ -2,7 +2,12 @@ import pytest from vfbquery.flybase_stocks import resolve_entity, find_stocks -from vfbquery.vfb_queries import get_flybase_stocks +from vfbquery.vfb_queries import ( + _flybase_report_url, + _md_link, + _stock_number_url, + get_flybase_stocks, +) def assert_single_selection_id(result): @@ -240,10 +245,67 @@ def test_stock_id_is_a_visible_column(self): # stock_id must be a normal displayed column, not the (hidden) identity. result = get_flybase_stocks(self.CONSTRUCT_WITH_STOCKS, return_dataframe=False, limit=3) stock_id = result["headers"]["stock_id"] - assert stock_id["type"] == "text" + assert stock_id["type"] == "markdown" assert stock_id["order"] >= 0 - # and the hidden identity carries the same FBst value - assert result["rows"][0]["id"] == result["rows"][0]["stock_id"] + # the hidden identity stays the bare FBst; the visible column links it + row = result["rows"][0] + assert row["stock_id"] == f"[{row['id']}](https://flybase.org/reports/{row['id']})" + + @pytest.mark.integration + def test_linked_columns_are_declared_markdown(self): + result = get_flybase_stocks(self.CONSTRUCT_WITH_STOCKS, return_dataframe=False, limit=3) + for col in ("stock_id", "stock_number", "collection"): + assert result["headers"][col]["type"] == "markdown", col + # genotype carries FlyBase bracket notation (w[1118]) and must stay text + assert result["headers"]["genotype"]["type"] == "text" + + @pytest.mark.integration + def test_collection_links_to_the_centre_homepage(self): + result = get_flybase_stocks(self.CONSTRUCT_WITH_STOCKS, return_dataframe=False, limit=3) + assert any(row["collection"].startswith("[") and "](" in row["collection"] + for row in result["rows"]) + + +class TestStockLinkouts: + """Unit tests for the linkout helpers — no database access.""" + + def test_flybase_report_url_accepts_any_fb_id(self): + assert _flybase_report_url("FBst0006565") == "https://flybase.org/reports/FBst0006565" + assert _flybase_report_url("FBrf0239740") == "https://flybase.org/reports/FBrf0239740" + + def test_flybase_report_url_rejects_non_ids(self): + assert _flybase_report_url("6565") is None + assert _flybase_report_url("") is None + assert _flybase_report_url(None) is None + + def test_md_link_without_url_is_plain_text(self): + assert _md_link("6565", None) == "6565" + assert _md_link("", "https://example.org/") == "" + + def test_md_link_escapes_parentheses_in_the_url(self): + # MarkdownLinkComponent's link target excludes ()[] so the label may + # contain brackets; a DOI with parentheses must not end the match early. + assert _md_link("10.1002/(SICI)1096", "https://doi.org/10.1002/(SICI)1096") == \ + "[10.1002/(SICI)1096](https://doi.org/10.1002/%28SICI%291096)" + + def test_bloomington_stock_number_deep_links(self): + assert _stock_number_url("Bloomington Drosophila Stock Center", "6565") == \ + "https://bdsc.indiana.edu/stocks/6565" + + def test_unknown_collection_has_no_stock_number_link(self, monkeypatch): + import vfbquery.flybase_stocks as fbs + monkeypatch.setattr(fbs, "collection_links", + lambda: {"Kyoto Stock Center": { + "order_url": "https://kyotofly.kit.jp/cgi-bin/stocks/index.cgi"}}) + assert _stock_number_url("Kyoto Stock Center", "103972") is None + + def test_order_url_ending_in_equals_takes_the_stock_number(self, monkeypatch): + import vfbquery.flybase_stocks as fbs + monkeypatch.setattr(fbs, "collection_links", + lambda: {"FlyORF": { + "order_url": "https://www.flyorf.ch/imlskonakart/SelectProd.do?flylineId="}}) + assert _stock_number_url("FlyORF", "F000748") == \ + "https://www.flyorf.ch/imlskonakart/SelectProd.do?flylineId=F000748" class TestFindStocksEdgeCases: diff --git a/src/vfbquery/flybase_stocks.py b/src/vfbquery/flybase_stocks.py index b266f97..4e50cad 100644 --- a/src/vfbquery/flybase_stocks.py +++ b/src/vfbquery/flybase_stocks.py @@ -519,3 +519,54 @@ def find_stocks(feature_id, collection_filter=None): return df.to_dict(orient="records") finally: conn.close() + + +# --------------------------------------------------------------------------- +# Stock centre linkouts +# --------------------------------------------------------------------------- + +# Chado carries the stock centres' own URLs in stockcollectionprop +# (homepage_url / order_url / request_text), which is what FlyBase's stock +# report renders. Read them rather than hard-coding a list that goes stale. +_COLLECTION_LINKS_SQL = """ +SELECT sc.uniquename AS collection, + c.name AS prop, + scp.value AS value +FROM stockcollection sc +JOIN stockcollectionprop scp ON sc.stockcollection_id = scp.stockcollection_id +JOIN cvterm c ON scp.type_id = c.cvterm_id +WHERE c.name IN ('homepage_url', 'order_url') +""" + +_collection_links_cache = None + + +def collection_links(): + """Return ``{collection_uniquename: {"homepage_url": ..., "order_url": ...}}``. + + Read once per process from chado's ``stockcollectionprop``. There are seven + collections and the values change about never, so a process-lifetime cache + is enough. A chado failure degrades to an empty map — callers then emit + plain text instead of a link, which is the pre-linkout behaviour. + + :return: dict keyed by stockcollection uniquename + """ + global _collection_links_cache + if _collection_links_cache is not None: + return _collection_links_cache + + links = {} + try: + conn = get_connection(statement_timeout_ms=15000) + try: + df = _run_query(conn, _COLLECTION_LINKS_SQL, {}) + finally: + conn.close() + for _, r in df.iterrows(): + links.setdefault(r["collection"], {})[r["prop"]] = r["value"] + except Exception as e: + print(f"Could not read stock collection links from FlyBase: {e}") + links = {} + + _collection_links_cache = links + return links diff --git a/src/vfbquery/vfb_queries.py b/src/vfbquery/vfb_queries.py index 2a2f5f9..347bcb9 100644 --- a/src/vfbquery/vfb_queries.py +++ b/src/vfbquery/vfb_queries.py @@ -9,7 +9,7 @@ from marshmallow import ValidationError import json import numpy as np -from urllib.parse import unquote +from urllib.parse import quote, unquote import hashlib from .solr_result_cache import (with_solr_cache, solr_caching_disabled, PREVIEW_STATUS_PENDING, PREVIEW_STATUS_COMPLETE, @@ -4944,6 +4944,69 @@ def get_upstream_class_connectivity(short_form: str, return_dataframe=True, limi return {'headers': headers, 'rows': rows, 'count': total_count} +# --------------------------------------------------------------------------- +# FlyBase linkout helpers (FindStocks / FindComboPublications) +# --------------------------------------------------------------------------- + +# Stock centres whose own catalogue has a stable per-stock URL that can be +# built from the FlyBase stock number. Chado carries every centre's homepage +# and order URL in stockcollectionprop, but not this one: Bloomington's +# order_url is its batch-order cart, so the per-stock pattern has to live here. +# FlyBase's own stock report deep-links the stock number for exactly two +# centres -- Bloomington and FlyORF -- and renders it as plain text for the +# other five, which only offer a search box (checked against one stock per +# collection, 2026-09-03). Mirror that rather than invent URL patterns. +_STOCK_NUMBER_URL = { + "Bloomington Drosophila Stock Center": "https://bdsc.indiana.edu/stocks/{number}", +} + +_FLYBASE_ID_RE = re.compile(r"^FB[a-z]{2}\d+$") + + +def _md_link(label, url): + """Build a ``[label](url)`` cell, or plain text when there is no url. + + Parentheses are percent-encoded in the url because the frontend's + MarkdownLinkComponent constrains the link target to ``[^()[\]]+`` so that + labels may contain brackets; a DOI such as ``10.1002/(SICI)...`` would + otherwise terminate the match early. Brackets in the label are left to + :func:`encode_markdown_links`. + """ + if label is None or label == "": + return "" + if not url: + return str(label) + safe_url = str(url).replace("(", "%28").replace(")", "%29") + return f"[{label}]({safe_url})" + + +def _flybase_report_url(fb_id): + """FlyBase report URL for an FB* id, or None if it is not one.""" + if isinstance(fb_id, str) and _FLYBASE_ID_RE.match(fb_id): + return f"https://flybase.org/reports/{fb_id}" + return None + + +def _stock_number_url(collection, number): + """URL for the stock centre's own catalogue entry, or None. + + Bloomington comes from the table above. Every other centre is derived from + chado: FlyORF's ``order_url`` is a per-line query prefix ending in ``=``, + so the stock number appends cleanly; the rest are homepages or batch-order + forms where appending a number would produce a dead link. + """ + if not collection or not number: + return None + pattern = _STOCK_NUMBER_URL.get(collection) + if pattern: + return pattern.format(number=quote(str(number), safe="")) + from .flybase_stocks import collection_links + order_url = collection_links().get(collection, {}).get("order_url") or "" + if order_url.endswith("="): + return order_url + quote(str(number), safe="") + return None + + def get_flybase_stocks(short_form: str, return_dataframe=True, limit: int = -1): """Find available fly stocks from FlyBase for a Feature term. @@ -4962,18 +5025,32 @@ def get_flybase_stocks(short_form: str, return_dataframe=True, limit: int = -1): return pd.DataFrame() return {'headers': {}, 'rows': [], 'count': 0} + from .flybase_stocks import collection_links + homepages = collection_links() + + # Three linkouts, matching what FlyBase's own stock report offers: the FBst + # id resolves to the FlyBase report (which carries the ordering details for + # every centre), the stock number resolves to the centre's own catalogue + # entry where that centre has one, and the collection name resolves to the + # centre's homepage. `id` stays the bare FBst — it is the row's selection + # id, not a rendered cell. rows = [] for s in stocks: + stock_id = s.get('stock_id', '') or '' + stock_number = s.get('stock_number', '') or '' + collection = s.get('collection', '') or '' rows.append({ # Hidden identity column (the FBst the row is about). Without a # `selection_id`-typed column the website consumes the first data # column as the row identity and hides it — which dropped Stock ID # from the table. See the `id` header below. - 'id': s.get('stock_id', ''), - 'stock_id': s.get('stock_id', ''), - 'stock_number': s.get('stock_number', ''), + 'id': stock_id, + 'stock_id': _md_link(stock_id, _flybase_report_url(stock_id)), + 'stock_number': _md_link( + stock_number, _stock_number_url(collection, stock_number)), 'genotype': s.get('genotype', ''), - 'collection': s.get('collection', ''), + 'collection': _md_link( + collection, homepages.get(collection, {}).get('homepage_url')), }) total_count = len(rows) @@ -4985,10 +5062,10 @@ def get_flybase_stocks(short_form: str, return_dataframe=True, limit: int = -1): headers = { 'id': {'title': 'ID', 'type': 'selection_id', 'order': -1}, - 'stock_id': {'title': 'Stock ID', 'type': 'text', 'order': 0}, - 'stock_number': {'title': 'Stock Number', 'type': 'text', 'order': 1}, + 'stock_id': {'title': 'Stock ID', 'type': 'markdown', 'order': 0}, + 'stock_number': {'title': 'Stock Number', 'type': 'markdown', 'order': 1}, 'genotype': {'title': 'Genotype', 'type': 'text', 'order': 2}, - 'collection': {'title': 'Collection', 'type': 'text', 'order': 3}, + 'collection': {'title': 'Collection', 'type': 'markdown', 'order': 3}, } return {'headers': headers, 'rows': rows, 'count': total_count} @@ -5011,21 +5088,31 @@ def get_flybase_combo_pubs(short_form: str, return_dataframe=True, limit: int = return pd.DataFrame() return {'headers': {}, 'rows': [], 'count': 0} + # Same three-linkout treatment as FindStocks: the FBrf resolves to its + # FlyBase report, and DOI / PMID / PMCID resolve to the publisher, PubMed + # and PMC respectively. Title, citation and type stay plain text. rows = [] for p in pubs: + fbrf = p.get('fbrf', '') or '' + doi = p.get('doi', '') or '' + pmid = p.get('pmid', '') or '' + pmcid = p.get('pmcid', '') or '' rows.append({ # Hidden identity column (the FBrf the row is about). Mirrors the # stocks fix: without a `selection_id` column the website consumes # the first data column (FBrf) as the row identity and hides it. - 'id': p.get('fbrf', ''), - 'fbrf': p.get('fbrf', ''), + 'id': fbrf, + 'fbrf': _md_link(fbrf, _flybase_report_url(fbrf)), 'title': p.get('title', ''), 'year': p.get('year', ''), 'miniref': p.get('miniref', ''), 'pub_type': p.get('pub_type', ''), - 'doi': p.get('doi', ''), - 'pmid': p.get('pmid', ''), - 'pmcid': p.get('pmcid', ''), + 'doi': _md_link(doi, f"https://doi.org/{doi}" if doi else None), + 'pmid': _md_link( + pmid, f"https://pubmed.ncbi.nlm.nih.gov/{pmid}/" if pmid else None), + 'pmcid': _md_link( + pmcid, + f"https://www.ncbi.nlm.nih.gov/pmc/articles/{pmcid}/" if pmcid else None), }) total_count = len(rows) @@ -5037,14 +5124,14 @@ def get_flybase_combo_pubs(short_form: str, return_dataframe=True, limit: int = headers = { 'id': {'title': 'ID', 'type': 'selection_id', 'order': -1}, - 'fbrf': {'title': 'FBrf', 'type': 'text', 'order': 0}, + 'fbrf': {'title': 'FBrf', 'type': 'markdown', 'order': 0}, 'title': {'title': 'Title', 'type': 'text', 'order': 1}, 'year': {'title': 'Year', 'type': 'text', 'order': 2}, 'miniref': {'title': 'Reference', 'type': 'text', 'order': 3}, 'pub_type': {'title': 'Type', 'type': 'text', 'order': 4}, - 'doi': {'title': 'DOI', 'type': 'text', 'order': 5}, - 'pmid': {'title': 'PMID', 'type': 'text', 'order': 6}, - 'pmcid': {'title': 'PMCID', 'type': 'text', 'order': 7}, + 'doi': {'title': 'DOI', 'type': 'markdown', 'order': 5}, + 'pmid': {'title': 'PMID', 'type': 'markdown', 'order': 6}, + 'pmcid': {'title': 'PMCID', 'type': 'markdown', 'order': 7}, } return {'headers': headers, 'rows': rows, 'count': total_count}