Skip to content

Restore documented answer set import behavior which was broken accidentally#1025

Merged
nonprofittechy merged 4 commits intomainfrom
answer-set-import-review
Mar 10, 2026
Merged

Restore documented answer set import behavior which was broken accidentally#1025
nonprofittechy merged 4 commits intomainfrom
answer-set-import-review

Conversation

@nonprofittechy
Copy link
Member

@nonprofittechy nonprofittechy commented Mar 10, 2026

09edd7d accidentally removed the menu item to allow answer set importing. This function needs more review, but it can be useful now on a development-only server when used by admins.

The commit looks a little funky because the old code had a few errors.

  1. Download current progress was not removed. It is still a separate menu item wired to the emergency document-
    download screen at al_visual.yml#L101, and that screen still renders the in-progress document downloads at
    al_visual.yml#L583.
  2. The latest commit changed the last menu item from a mislabeled Download answer set to a correctly labeled
    Import answer set. The new menu entry now points to the JSON upload flow at al_visual.yml#L113, and the label
    text was updated at al_visual.yml#L144. That upload screen is the JSON import screen at
    al_saved_sessions.yml#L232.
  3. Before this commit, that same menu item was already wrong: it said Download answer set but actually opened the
    emergency document download screen, not an answer-set export flow. So the latest change did not overwrite a
    working download screen; it fixed a long-standing miswire.
  4. There is still no dedicated main-menu Download answer set item. JSON export does exist, but it is exposed from
    the share flow at al_visual.yml#L292 and from the saved-answer-set detail screen at
    al_saved_sessions_store.yml#L41.

I consider this an experimental feature that should only be used by developers, and that's how we describe it everywhere in our documentation right now. The installer also does not turn this feature on by default. Server admins need to opt-in to it.

Therefore, restoring this should be low risk.

I'm working on a larger security plan to beef this feature's safety up.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR restores the "Import answer set" menu item that was accidentally removed in a prior commit. The menu entry now correctly points to the JSON upload flow (defined in al_saved_sessions.yml) instead of the emergency document-download screen it was previously miswired to. The label was also corrected from "Download answer set" to "Import answer set", and an admin bypass was added so that logged-in admins can always access the import feature.

Changes:

  • Replaced the URL from url_action('al_error_action_download_screen') to url_ask(["al_sessions_json_file", ...]) to correctly wire the menu item to the JSON import flow.
  • Updated the hidden condition to require both enable answer sets and enable answer set imports config flags, with an admin bypass for logged-in admins.
  • Changed the al_download_answer_set_string template content from "Download answer set" to "Import answer set" to accurately reflect the action.

Copy link
Contributor

Copilot AI commented Mar 10, 2026

@nonprofittechy I've opened a new pull request, #1026, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits March 10, 2026 16:06
…or consistency

Co-authored-by: nonprofittechy <7645641+nonprofittechy@users.noreply.github.com>
Rename `al_download_answer_set_string` to `al_import_answer_set_string`
Copy link
Contributor

@BryceStevenWilley BryceStevenWilley left a comment

Choose a reason for hiding this comment

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

Looks fine to me. I'll be sure to do a much deeper review on the security beef-up you're planning ;)

@nonprofittechy nonprofittechy merged commit 69ef6db into main Mar 10, 2026
9 checks passed
@nonprofittechy nonprofittechy deleted the answer-set-import-review branch March 10, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants