feat: Organize global admin settings form into tabs for better UX #591
Conversation
Reviewer's Guide by SourceryThis pull request restructures the Global Admin Settings interface by introducing a tabbed layout. The settings are now organized into five categories: Basics, Localization, Email, Payment Gateways, and Maps. This change aims to improve the user experience by providing a more organized and intuitive interface. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @SxxAq - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Flagging hardcoded password field in global settings form. (link)
- Flagging hardcoded stripe webhook secret key field in global settings form. (link)
- Flagging hardcoded send grid api key field in global settings form. (link)
- Flagging hardcoded stripe connect secret key field in global settings form. (link)
Overall Comments:
- Consider using a template tag or filter to generate the tab URLs to avoid hardcoding them in the template.
- The javascript code to show/hide email vendor specific fields should be moved to a separate file.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🔴 Security: 4 blocking issues
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } else if (thrownError !== "abort" && thrownError !== "") { | ||
| console.log(thrownError); | ||
| alert(gettext("Unknown error.")); | ||
| } | ||
| }); |
There was a problem hiding this comment.
issue (complexity): Consider refactoring repeated configuration blocks for datetimepickers, colorpickers, and select2 by extracting shared settings into helper methods to reduce complexity and improve code maintainability.
Refactoring the repeated configuration blocks can reduce complexity without changing behavior. For example, extract shared settings for datetimepickers and colorpickers into helper methods. One approach would be:
function getDateTimePickerDefaults(extra) {
const defaults = {
format: $("body").attr("data-datetimeformat"),
locale: $("body").attr("data-datetimelocale"),
useCurrent: false,
showClear: true, // update based on element requirement later if needed
icons: {
time: "fa fa-clock-o",
date: "fa fa-calendar",
up: "fa fa-chevron-up",
down: "fa fa-chevron-down",
previous: "fa fa-chevron-left",
next: "fa fa-chevron-right",
today: "fa fa-screenshot",
clear: "fa fa-trash",
close: "fa fa-remove",
}
};
return $.extend({}, defaults, extra);
}
// Usage in form_handlers:
el.find(".datetimepicker").each(function () {
$(this).datetimepicker(
getDateTimePickerDefaults({showClear: !$(this).prop("required")})
);
if (!$(this).val()) {
$(this).data("DateTimePicker").viewDate(moment().startOf("day"));
}
});Similarly, create helper methods for settings used in colorpicker and select2, etc. This keeps functionality intact while reducing nesting and repetition.
src/pretix/control/urls.py
Outdated
| # other custom paths can be added here | ||
| ], | ||
| "oauth2_provider.subdomain", | ||
| ) | ||
|
|
||
| urlpatterns = [ |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the URL routes into dedicated modules to improve maintainability and reduce complexity by grouping related routes into separate files and including them in the main urls.py file.
Refactoring the URL routes into dedicated modules would reduce vertical space and nesting, making maintenance easier. Consider grouping related routes (e.g. admin, event, oauth) into separate files and then including them in the main urls.py.
For example:
# pretix/control/urls/admin_urls.py
from django.urls import re_path as url
from pretix.control.views import admin, pages, global_settings, user, users, typeahead
urlpatterns = [
url(r"^$", admin.AdminDashboard.as_view(), name="admin.dashboard"),
url(r"^organizers/$", admin.OrganizerList.as_view(), name="admin.organizers"),
# ... other admin routes
]Then update your main file:
# pretix/control/urls.py
from django.urls import include, re_path as url
urlpatterns = [
url(r"^admin/", include("pretix.control.urls.admin_urls")),
# ... include other route groups similarly
]This retains all functionality while easing navigation and reducing complexity in a single file.
| "smtp_host", | ||
| "smtp_port", | ||
| "smtp_username", | ||
| "smtp_password", |
There was a problem hiding this comment.
🚨 issue (security): Flagging hardcoded password field in global settings form.
| "payment_stripe_connect_publishable_key", | ||
| "payment_stripe_connect_test_secret_key", | ||
| "payment_stripe_connect_test_publishable_key", | ||
| "stripe_webhook_secret_key", |
There was a problem hiding this comment.
🚨 issue (security): Flagging hardcoded stripe webhook secret key field in global settings form.
| "email": [ | ||
| "mail_from", | ||
| "email_vendor", | ||
| "send_grid_api_key", |
There was a problem hiding this comment.
🚨 issue (security): Flagging hardcoded send grid api key field in global settings form.
| "smtp_use_ssl", | ||
| ], | ||
| "payment_gateways": [ | ||
| "payment_stripe_connect_secret_key", |
There was a problem hiding this comment.
🚨 issue (security): Flagging hardcoded stripe connect secret key field in global settings form.
| if ( | ||
| global_settings.get("smtp_port") is None | ||
| or global_settings.get("smtp_port") == "" | ||
| ): |
There was a problem hiding this comment.
You seem to have a different line length setting, which introduces a lot of unrelated changes.
Please change this and make sure that no code changes are mixed with reformatting issues.
| ), | ||
| ( | ||
| "banner_message", | ||
| I18nFormField( |
There was a problem hiding this comment.
Same in all the changes here, unrelated reformatting changes
|
@norbusan I have resolved the formatting-related issues. Please review the changes. |
Description
This PR implements the redesign of Global Admin Settings to align with the event settings UI pattern. The new design incorporates a tabbed interface that improves organization and user experience for administrators.
Changes Made
Restructured the Global Admin Settings interface to use a tabbed layout
Organized settings into five distinct categories:
Screenshots
Related Issue
🎯 Closes #479
Summary by Sourcery
Organize the global admin settings form into tabs for better user experience. This change restructures the global admin settings interface to use a tabbed layout, organizing settings into five distinct categories: Basics, Localization, Email, Payment Gateways, and Maps.
Enhancements: