Acp badges rewrite - Githubissues

ghost commented 6 years ago

A full rewrite of the ACP badges module with changes to the template and most language keys. The module info has also been changed which probably requires to delete the extension data to be safe. Otherwise the changes to the module info will break phpbb. We will have to repeat this for the foreseeable changes of the UCP module info too. Reason for the module info changes are foreseeable problems when adding more modules.

Notable changes in badges_module.php:

Actions are parameters sent via GET that define a context for POST submits
Define a context for adding badges/ badge types in a GET request instead of using a POST submit to show the form.
Switch over modes and switch over action for each. Define submits within the actions.
Remove most SELECT COUNT(*) queries
Fix some errors where $u_action was not defined correctly and would refer to wrong actions / parameters (the "Return to last page" link does not work everywhere as intended.)

I restructured the code quite heavily so the diff is not really helpful I fear.

pchote commented 6 years ago

Deleting the extension data isn't an option, we can't throw out all the user data.

ghost commented 6 years ago

I'll find a way to handle this.

ghost commented 6 years ago

Updated. For updating the extension, disable the running installation, apply the changes and enable it again. The migration file will rename the module and modes. Tested the migration from 1.0.1 to 1.0.2 several times without any problems. It will also work if there is not a running installation (first install) and deleting all extension data will work after the migration too.

I have made some more changes and will leave the pr in this state.

ghost commented 6 years ago

I have some other ideas and can't say how or if they affect these changes and it is probably best to wait with any reviews until I have decided how to proceed.

ghost commented 6 years ago

Nothing will happen here in the near future.

pchote commented 6 years ago

Reopening, as I still plan to review and merge this once time permits.

ghost commented 6 years ago

Ok, good to know.

ghost commented 5 years ago

See https://github.com/OpenRA/openrauseraccounts/pull/39#issuecomment-443545899. This will be superseded by another PR.

OpenRA / openrauseraccounts

Acp badges rewrite #38