Skip to content

FIREFLY-2076: Fix charts not showing for table/chart IDs containing a period - #2054

Merged
jaladh-singhal merged 2 commits into
devfrom
FIREFLY-2076-period-in-id-bug
Oct 29, 2026
Merged

jaladh-singhal merged 2 commits into
devfrom
FIREFLY-2076-period-in-id-bug

Conversation

@jaladh-singhal

Copy link
Copy Markdown
Member

Fixes FIREFLY-2076

A period in a table or chart ID (e.g. tbl_id='2mass.catalog' from the Python client) left the table's default chart empty, and show_chart(chartId='color.mag') drew a blank chart. Lodash string paths treat . as nesting, so updateSet(state, chartId, …) in ChartsCntlr stored the chart under state['default-2mass'].catalog while readers looked up the whole key; TableStatsCntlr had the same mismatch for column stats. Image plot_ids and region layer IDs with periods were checked and already work. No firefly_client changes are needed: IDs with periods now work as-is.

See the first commit's diff for the fix (array paths, plus CSS.escape for the chart Save button's #${chartId} selector) and dotted-ID reducer tests.

Additional refactor: The second commit (separately revertable) replaces lodash get/set/updateSet with ?., [] and spreads in those reducers and chart getters, keeping no-op CHART_UPDATE/CHART_TRACE_REMOVE returning the same state.

Testing

https://firefly-2076-period-in-id-bug.irsakubedev.ipac.caltech.edu/firefly/

Using firefly_client pointed at the URL above (fc = FireflyClient.make_client(url=..., launch_browser=False), then open fc.get_firefly_url()):

  • fc.show_table("http://irsa.ipac.caltech.edu/TAP/sync?FORMAT=IPAC_TABLE&QUERY=SELECT+*+FROM+fp_psc+WHERE+CONTAINS(POINT('J2000',ra,dec),CIRCLE('J2000',202.4841667,47.23055556,0.125))=1", tbl_id='2mass.catalog') → Active Chart shows the default chart (empty before this fix); table, coverage and catalog overlay as usual
  • fc.show_chart(data=[{'tbl_id': '2mass.catalog', 'x': 'tables::j_m', 'y': 'tables::h_m-k_m', 'type': 'scatter', 'mode': 'markers'}], chartId='color.mag') → chart renders under Pinned Chart
  • On that chart: Save button opens the Save Chart dialog; selecting a table row moves the highlighted point; Chart Options → change X → Apply updates the axis
  • Remove the table → its charts are cleaned up, no console errors
  • Regression: the same steps with IDs without periods

@stargaser stargaser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it and it works great! Thank you for this fix.

@loitly loitly left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good.

@jaladh-singhal
jaladh-singhal merged commit de49745 into dev Oct 29, 2026
2 checks passed
@jaladh-singhal
jaladh-singhal deleted the FIREFLY-2076-period-in-id-bug branch October 29, 2026 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Charts Anything related to charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants