search

widelands / wl_addons_server

Provides the add-ons server and all add-ons for the Widelands game.
https://www.widelands.org
Other
6 stars 4 forks source link

Update Add-On Notice Types Info #128

Closed Noordfrees closed 2 years ago

Noordfrees commented 2 years ago

This repo's side of https://github.com/widelands/widelands-website/pull/374

This should be tested carefully on alpha before merging, however alpha is currently running #127

Btw I already created the noticetypes on the website, however the settings there are ignored until this PR is merged.

Noordfrees commented 2 years ago

This is up on alpha now. I changed the noticetype slugs on alpha to what this branch expects.

Alpha website is also up so you can play with your notice settings.

frankystone commented 2 years ago

Do edited comments send again?

Noordfrees commented 2 years ago

Only to users who are @mentioned in the edited version but not in the original. Otherwise it would be e-mail spamming IMHO since it's not uncommon to edit a comment multiple times. I tried to keep this consistent with forum and GitHub notifications.

frankystone commented 2 years ago

Wrote some comments with mentioning you to 'auto_soldiers_cs.wad'. Do you get emails?

frankystone commented 2 years ago

Hm, wrong spelling of your name i think.

frankystone commented 2 years ago

Looks like everything is working fine :+1:

It would be nice to have a popup of usernames if one writes @+Letter... to prevent misspelling ;) New comments are only shown if one leaves the 'Add-Ons'-section and click on Add-Ons in the main menu again. But i think this can be ignored.

Noordfrees commented 2 years ago

It would be nice to have a popup of usernames if one writes @+Letter... to prevent misspelling ;)

+1 but for this to work the server would need to send the complete list of all registered usernames to the client once. Communicating after every keystroke instead would be very slow. Do we want this?

New comments are only shown if one leaves the 'Add-Ons'-section and click on Add-Ons in the main menu again. But i think this can be ignored.

Alternatively you can click the Refresh Remotes button to update the add-ons info

frankystone commented 2 years ago

It would be nice to have a popup of usernames if one writes @+Letter... to prevent misspelling ;)

+1 but for this to work the server would need to send the complete list of all registered usernames to the client once. Communicating after every keystroke instead would be very slow. Do we want this?

We have similar functionality on the website which calls a function to query the users database if at least 3 Characters are written. This reduces the amount of available usernames. But i don't know if this is feasable for the addons server because the ajax connection on the website is provided by jquery. See: https://github.com/widelands/widelands-website/blob/master/django_messages_wl/views.py and: https://github.com/widelands/widelands-website/blob/dd9f35559a7726d9f597237662c0df064bec6b9b/templates/django_messages/compose.html#L10-L16

frankystone commented 2 years ago

This has been heavily tested on alpha and should be good to go.

Anyway i am a bit concerned about the hardcoded values. This means every time a notice type's slug changes the code has to be adopted. This is also difficult if there are other slugs on alpha than on the productive system. Can't there be an other config file which defines the mappings of slug <-> notice type? So there can be other slugs for alpha than in productive.

Noordfrees commented 2 years ago

Thanks for the review and testing :) Merged and deployed.