Skip to content

Fix: Votemap Whitelist page errors when whitelist contains non-existing#1202

Merged
Dorfieeee merged 3 commits intomasterfrom
issue/old_map_ids
Mar 25, 2026
Merged

Fix: Votemap Whitelist page errors when whitelist contains non-existing#1202
Dorfieeee merged 3 commits intomasterfrom
issue/old_map_ids

Conversation

@FlorianSW
Copy link
Copy Markdown
Collaborator

map

With the recent update, Stalingrad was renamed to a different map ID, however, Votemap whitelists might still contain the old IDs of the map. When that is the case, the votemap whitelist page simply errors, as these maps will be replaced with "undefined" instead of their map object counterpart.

I decided to simply remove the map IDs from the votemap whitelist selected maps list. An alternative would've been to make a migration server-side to replace the old map ID with the new one.

map

With the recent update, Stalingrad was renamed to a different map ID,
however, Votemap whitelists might still contain the old IDs of the map.
When that is the case, the votemap whitelist page simply errors, as
these maps will be replaced with "undefined" instead of their map object
counterpart.

I decided to simply remove the map IDs from the votemap whitelist
selected maps list. An alternative would've been to make a migration
server-side to replace the old map ID with the new one.
@Dorfieeee
Copy link
Copy Markdown
Collaborator

I am not sure right now how votemap behaves when invalid map is given but yeah I think it would be much better to make a check while running the maintenance container so ensure that neither autosettings nor votemap or anything else does not contain some old ids and other values.

@FlorianSW
Copy link
Copy Markdown
Collaborator Author

Sorry for my late response 🙈

I am not sure right now how votemap behaves when invalid map is given but yeah I think it would be much better to make a check while running the maintenance container so ensure that neither autosettings nor votemap or anything else does not contain some old ids and other values.

Agreed for the Votemap part. The autosettings part shouldn't be an issue. It's "just" a config to invoke API commands. Hence it should be the task of the api command to handle cases where "old" map IDs are still put in. Furthermore I feel that touching the autosettings config is an overkill for this (with all the potential parsing and mapping of API commands and map lists) for a potential issue that is limited.

I added a task to remove orphaned VoteMap IDs from the config in the latest commit. I also changed how the whitelist is persisted in redis. Before this, there were multiple ways on how the list was saved, either as a list of layers, or a list of strings, or a combination of both. That required several places to parse the list of maps, even though the type information already suggested that it is a set of parsed map layers. I made sure that we always only save the map IDs and parse the them when getting them back from redis. In that way the API can always safely expect that the type a method returns/accepts is acftually the contents of the value.

@FlorianSW
Copy link
Copy Markdown
Collaborator Author

@Dorfieeee any feedback? Can we merge that? :)

@Dorfieeee
Copy link
Copy Markdown
Collaborator

I haven't had chance to run it @FlorianSW but it looks okay to me. However I would be happier if this change was based on #1064 which could be also finally merged

Copy link
Copy Markdown
Collaborator

@Dorfieeee Dorfieeee left a comment

Choose a reason for hiding this comment

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

👍🏻

Copy link
Copy Markdown
Collaborator

@Dorfieeee Dorfieeee left a comment

Choose a reason for hiding this comment

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

👍🏻

@Dorfieeee Dorfieeee merged commit 8f08a6c into master Mar 25, 2026
3 checks passed
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.

2 participants