Repository navigation
Conversation
A URL typed into the WMS URL box and confirmed with Enter was never stored: Enter inserts it into the combo box, so the success handler found it there and skipped saving. Only one URL was stored anyway. msui now keeps every URL it loaded capabilities from in a list shared by all views, most recent first, and still reads the single URL older versions stored. Del or Backspace on a highlighted URL in the opened list forgets it; default servers stay, and a loaded server stays listed until it is unloaded. Also fix the success handler leaving duplicate entries when a known URL is entered again without trailing slash, and add_wms_urls adding duplicates within its list. The tests reset the stored URLs before every test. Refs #3317 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
We might need to discuss this. The behaviour of MSS was not ideal when we configured it for the campaign yesterday. |
There was a problem hiding this comment.
Hi Jörn
some hints by claude code-review, commented by me
#3318 fixes only part of #3317. The reported bug is fixed: a newly entered WMS URL now stays in the list. The issue's second request isn't done, which is to offer, on the first connection, to add the server to the default list (default_WMS in msui_settings.json). The PR still says "Fixes #3317", so merging it would close the issue with that part undone. joernu76 already wrote on the PR "We might need to discuss this". Either the second part goes into a follow-up issue, or the PR should say "Refs #3317" instead.
What the PR gets right:
- The cause is identified correctly: pressing Enter put the typed URL into the list, so the success handler found it there and never saved it.
- Every successful server connection now saves its URL, whatever the list shows.
- The single URL that older versions stored is still read.
Bugs worth raising:
- Already open views don't see new or deleted URLs (wms_control.py:574). Each view reads the list once, when it opens. Load a new server in the top view, and an open side view won't list it until reopened. Delete a URL in one view, and it stays in the other. The docs, though, say the list is shared by all views.
- Default URLs can disappear (wms_control.py:1035). When the server answers with a slightly different address, e.g. a trailing /, the code removes or renames the entry that was used, even when it's a configured default. A renamed default can then be deleted with Del.
- Deleting with Del can seem to do nothing (wms_control.py:1063). If the deleted entry was the selected one, its URL stays in the input field. The next click on Get Capabilities loads it and saves it again.
- Secrets in URLs are kept indefinitely (wms_control.py:80). URLs are saved in full, with user, password or token, in plain text in msui-core.conf. Before, only the last URL was kept and overwritten each time. Now they stay until the user deletes each one with Del, which only a tooltip mentions.
- The list has no size limit and fills with URLs the user never typed (wms_control.py:80), such as autoplot servers and redirect targets.
- All views share one list (wms_control.py:573). Map servers loaded in the top view also show up in the side and linear views, where they offer no layers.
Smaller:
7. A URL list stored as a plain string falls apart into single characters (wms_control.py:71): list() is applied without a type check.
8. Saving the list deletes the old key recent_wms_url (wms_control.py:81). An 11.x installation using the same config file then loses its remembered URL.
9. select_wms_url looks up the same URL up to three times (wms_control.py:1028). This tangled logic is where bug 2 is.
For 1. we maybe need to update the lists on open views
For 2. and 3. and 9. we need a test and then a fix
For 4. We should exclude Urls with API Keys, passwords, token.
For 5. we should add a limit e.g. 20? and cycle then.
For 6. we maybe should have a check of valid data for the view. We should not show servers which don't provide data.
A URL typed into the WMS URL box and confirmed with Enter was never stored: Enter inserts it into the combo box, so the success handler found it there and skipped saving. Only one URL was stored anyway.
msui now keeps every URL it loaded capabilities from in a list shared by all views, most recent first, and still reads the single URL older versions stored. Del or Backspace on a highlighted URL in the opened list forgets it; default servers stay, and a loaded server stays listed until it is unloaded.
Also fix the success handler leaving duplicate entries when a known URL is entered again without trailing slash, and add_wms_urls adding duplicates within its list. The tests reset the stored URLs before every test.
Refs #3317
Purpose of PR?:
Fixes #3317