feat: show alert/prompt when settings changes require restart (#2401)

* fix: correct 'StatusChecker' typo

* feat: add restart required check to StatusChecker

* fix(perms): remove MANAGE_SETTINGS permission

* fix: allow alert to be dismissed

* fix(lang): add missing string in SettingsServices

* fix(frontend): fix modal icon border

* fix(frontend): un-dismiss alert if setting reverted not require server restart

* fix(backend): restart flag only needs to track main settings

* fix: rebase issue

* refactor: appease Prettier

* refactor: swap settings badge order

* fix: type import for MainSettings

* test: add cypress test for restart prompt

cypress/e2e/settings/general-settings.cy.ts

@@ -0,0 +1,32 @@
+describe('General Settings', () => {
+  beforeEach(() => {
+    cy.login(Cypress.env('ADMIN_EMAIL'), Cypress.env('ADMIN_PASSWORD'));
+  });
+
+  it('opens the settings page from the home page', () => {
+    cy.visit('/');
+
+    cy.get('[data-testid=sidebar-toggle]').click();
+    cy.get('[data-testid=sidebar-menu-settings-mobile]').click();
+
+    cy.get('.heading').should('contain', 'General Settings');
+  });
+
+  it('modifies setting that requires restart', () => {
+    cy.visit('/settings');
+
+    cy.get('#trustProxy').click();
+    cy.get('form').submit();
+    cy.get('[data-testid=modal-title]').should(
+      'contain',
+      'Server Restart Required'
+    );
+
+    cy.get('[data-testid=modal-ok-button]').click();
+    cy.get('[data-testid=modal-title]').should('not.exist');
+
+    cy.get('[type=checkbox]#trustProxy').click();
+    cy.get('form').submit();
+    cy.get('[data-testid=modal-title]').should('not.exist');
+  });
+});

cypress/e2e/user/user-list.cy.ts

@@ -15,7 +15,7 @@ describe('User List', () => {
     cy.get('[data-testid=sidebar-toggle]').click();
     cy.get('[data-testid=sidebar-menu-users-mobile]').click();
 
-    cy.get('[data-testid=page-header').should('contain', 'User List');
+    cy.get('[data-testid=page-header]').should('contain', 'User List');
   });
 
   it('can find the admin user and friend user in the user list', () => {
@@ -30,15 +30,15 @@ describe('User List', () => {
 
     cy.contains('Create Local User').click();
 
-    cy.get('[data-testid=modal-title').should('contain', 'Create Local User');
+    cy.get('[data-testid=modal-title]').should('contain', 'Create Local User');
 
     cy.get('#displayName').type(testUser.displayName);
     cy.get('#email').type(testUser.emailAddress);
     cy.get('#password').type(testUser.password);
 
     cy.intercept('/api/v1/user?take=10&skip=0&sort=displayname').as('user');
 
-    cy.get('[data-testid=modal-ok-button').click();
+    cy.get('[data-testid=modal-ok-button]').click();
 
     cy.wait('@user');
     // Wait a little longer for the user list to fully re-render
@@ -58,7 +58,7 @@ describe('User List', () => {
 
     cy.intercept('/api/v1/user?take=10&skip=0&sort=displayname').as('user');
 
-    cy.get('[data-testid=modal-ok-button').should('contain', 'Delete').click();
+    cy.get('[data-testid=modal-ok-button]').should('contain', 'Delete').click();
 
     cy.wait('@user');
     cy.wait(1000);

overseerr-api.yml

@@ -1793,14 +1793,14 @@ components:
 paths:
   /status:
     get:
-      summary: Get Overseerr version
-      description: Returns the current Overseerr version in a JSON object.
+      summary: Get Overseerr status
+      description: Returns the current Overseerr status in a JSON object.
       security: []
       tags:
         - public
       responses:
         '200':
-          description: Returned version
+          description: Returned status
           content:
             application/json:
               schema:
@@ -1811,6 +1811,12 @@ paths:
                     example: 1.0.0
                   commitTag:
                     type: string
+                  updateAvailable:
+                    type: boolean
+                  commitsBehind:
+                    type: number
+                  restartRequired:
+                    type: boolean
   /status/appdata:
     get:
       summary: Get application data volume status

server/index.ts

@@ -31,6 +31,7 @@ import { getSettings } from './lib/settings';
 import logger from './logger';
 import routes from './routes';
 import { getAppVersion } from './utils/appVersion';
+import restartFlag from './utils/restartFlag';
 
 const API_SPEC_PATH = path.join(__dirname, '../overseerr-api.yml');
 
@@ -53,6 +54,7 @@ app
 
     // Load Settings
     const settings = getSettings().load();
+    restartFlag.initializeSettings(settings.main);
 
     // Migrate library types
     if (

server/interfaces/api/settingsInterfaces.ts

@@ -56,4 +56,5 @@ export interface StatusResponse {
   commitTag: string;
   updateAvailable: boolean;
   commitsBehind: number;
+  restartRequired: boolean;
 }

server/lib/permissions.ts

@@ -1,7 +1,6 @@
 export enum Permission {
   NONE = 0,
   ADMIN = 2,
-  MANAGE_SETTINGS = 4,
   MANAGE_USERS = 8,
   MANAGE_REQUESTS = 16,
   REQUEST = 32,

server/routes/index.ts

@@ -14,6 +14,7 @@ import { mapProductionCompany } from '../models/Movie';
 import { mapNetwork } from '../models/Tv';
 import { appDataPath, appDataStatus } from '../utils/appDataVolume';
 import { getAppVersion, getCommitTag } from '../utils/appVersion';
+import restartFlag from '../utils/restartFlag';
 import { isPerson } from '../utils/typeHelpers';
 import authRoutes from './auth';
 import collectionRoutes from './collection';
@@ -78,6 +79,7 @@ router.get<unknown, StatusResponse>('/status', async (req, res) => {
     commitTag: getCommitTag(),
     updateAvailable,
     commitsBehind,
+    restartRequired: restartFlag.isSet(),
   });
 });
 
@@ -100,11 +102,7 @@ router.get('/settings/public', async (req, res) => {
     return res.status(200).json(settings.fullPublicSettings);
   }
 });
-router.use(
-  '/settings',
-  isAuthenticated(Permission.MANAGE_SETTINGS),
-  settingsRoutes
-);
+router.use('/settings', isAuthenticated(Permission.ADMIN), settingsRoutes);
 router.use('/search', isAuthenticated(), searchRoutes);
 router.use('/discover', isAuthenticated(), discoverRoutes);
 router.use('/request', isAuthenticated(), requestRoutes);

server/routes/user/index.ts

@@ -258,12 +258,7 @@ export const canMakePermissionsChange = (
   user?: User
 ): boolean =>
   // Only let the owner grant admin privileges
-  !(hasPermission(Permission.ADMIN, permissions) && user?.id !== 1) ||
-  // Only let users with the manage settings permission, grant the same permission
-  !(
-    hasPermission(Permission.MANAGE_SETTINGS, permissions) &&
-    !hasPermission(Permission.MANAGE_SETTINGS, user?.permissions ?? 0)
-  );
+  !(hasPermission(Permission.ADMIN, permissions) && user?.id !== 1);
 
 router.put<
   Record<string, never>,

server/utils/restartFlag.ts

@@ -0,0 +1,23 @@
+import type { MainSettings } from '../lib/settings';
+import { getSettings } from '../lib/settings';
+
+class RestartFlag {
+  private settings: MainSettings;
+
+  public initializeSettings(settings: MainSettings): void {
+    this.settings = { ...settings };
+  }
+
+  public isSet(): boolean {
+    const settings = getSettings().main;
+
+    return (
+      this.settings.csrfProtection !== settings.csrfProtection ||
+      this.settings.trustProxy !== settings.trustProxy
+    );
+  }
+}
+
+const restartFlag = new RestartFlag();
+
+export default restartFlag;

src/components/Common/Modal/index.tsx

@@ -130,7 +130,7 @@ const Modal: React.FC<ModalProps> = ({
               />
             </div>
           )}
-          <div className="relative overflow-x-hidden sm:flex sm:items-center">
+          <div className="relative overflow-x-hidden p-0.5 sm:flex sm:items-center">
             {iconSvg && <div className="modal-icon">{iconSvg}</div>}
             <div
               className={`mt-3 truncate text-center text-white sm:mt-0 sm:text-left ${
@@ -149,7 +149,11 @@ const Modal: React.FC<ModalProps> = ({
             </div>
           </div>
           {children && (
-            <div className="relative mt-4 text-sm leading-5 text-gray-300">
+            <div
+              className={`relative mt-4 text-sm leading-5 text-gray-300 ${
+                !(onCancel || onOk || onSecondary || onTertiary) ? 'mb-3' : ''
+              }`}
+            >
               {children}
             </div>
           )}

src/components/Layout/Sidebar/index.tsx

@@ -80,7 +80,8 @@ const SidebarLinks: SidebarLinkProps[] = [
     messagesKey: 'settings',
     svgIcon: <CogIcon className="mr-3 h-6 w-6" />,
     activeRegExp: /^\/settings/,
-    requiredPermission: Permission.MANAGE_SETTINGS,
+    requiredPermission: Permission.ADMIN,
+    dataTestId: 'sidebar-menu-settings',
   },
 ];
 

src/components/PermissionEdit/index.tsx

@@ -12,9 +12,6 @@ export const messages = defineMessages({
   users: 'Manage Users',
   usersDescription:
     'Grant permission to manage users. Users with this permission cannot modify users with or grant the Admin privilege.',
-  settings: 'Manage Settings',
-  settingsDescription:
-    'Grant permission to modify global settings. A user must have this permission to grant it to others.',
   managerequests: 'Manage Requests',
   managerequestsDescription:
     'Grant permission to manage media requests. All requests made by a user with this permission will be automatically approved.',
@@ -88,12 +85,6 @@ export const PermissionEdit: React.FC<PermissionEditProps> = ({
       description: intl.formatMessage(messages.adminDescription),
       permission: Permission.ADMIN,
     },
-    {
-      id: 'settings',
-      name: intl.formatMessage(messages.settings),
-      description: intl.formatMessage(messages.settingsDescription),
-      permission: Permission.MANAGE_SETTINGS,
-    },
     {
       id: 'users',
       name: intl.formatMessage(messages.users),

src/components/PermissionOption/index.tsx

@@ -67,14 +67,9 @@ const PermissionOption: React.FC<PermissionOptionProps> = ({
   }
 
   if (
-    // Non-Admin users cannot modify the Admin permission
-    (actingUser &&
-      !hasPermission(Permission.ADMIN, actingUser.permissions) &&
-      option.permission === Permission.ADMIN) ||
-    // Users without the Manage Settings permission cannot modify/grant that permission
-    (actingUser &&
-      !hasPermission(Permission.MANAGE_SETTINGS, actingUser.permissions) &&
-      option.permission === Permission.MANAGE_SETTINGS)
+    // Only the owner can modify the Admin permission
+    actingUser?.id !== 1 &&
+    option.permission === Permission.ADMIN
   ) {
     disabled = true;
   }

src/components/Settings/SettingsMain.tsx

@@ -41,16 +41,15 @@ const messages = defineMessages({
   toastSettingsFailure: 'Something went wrong while saving settings.',
   hideAvailable: 'Hide Available Media',
   csrfProtection: 'Enable CSRF Protection',
-  csrfProtectionTip:
-    'Set external API access to read-only (requires HTTPS, and Overseerr must be reloaded for changes to take effect)',
+  csrfProtectionTip: 'Set external API access to read-only (requires HTTPS)',
   csrfProtectionHoverTip:
     'Do NOT enable this setting unless you understand what you are doing!',
   cacheImages: 'Enable Image Caching',
   cacheImagesTip:
     'Optimize and store all images locally (consumes a significant amount of disk space)',
   trustProxy: 'Enable Proxy Support',
   trustProxyTip:
-    'Allow Overseerr to correctly register client IP addresses behind a proxy (Overseerr must be reloaded for changes to take effect)',
+    'Allow Overseerr to correctly register client IP addresses behind a proxy',
   validationApplicationTitle: 'You must provide an application title',
   validationApplicationUrl: 'You must provide a valid URL',
   validationApplicationUrlTrailingSlash: 'URL must not end in a trailing slash',
@@ -151,6 +150,7 @@ const SettingsMain: React.FC = () => {
                 trustProxy: values.trustProxy,
               });
               mutate('/api/v1/settings/public');
+              mutate('/api/v1/status');
 
               if (setLocale) {
                 setLocale(
@@ -252,7 +252,12 @@ const SettingsMain: React.FC = () => {
                 </div>
                 <div className="form-row">
                   <label htmlFor="trustProxy" className="checkbox-label">
-                    <span>{intl.formatMessage(messages.trustProxy)}</span>
+                    <span className="mr-2">
+                      {intl.formatMessage(messages.trustProxy)}
+                    </span>
+                    <Badge badgeType="primary">
+                      {intl.formatMessage(globalMessages.restartRequired)}
+                    </Badge>
                     <span className="label-tip">
                       {intl.formatMessage(messages.trustProxyTip)}
                     </span>
@@ -273,9 +278,12 @@ const SettingsMain: React.FC = () => {
                     <span className="mr-2">
                       {intl.formatMessage(messages.csrfProtection)}
                     </span>
-                    <Badge badgeType="danger">
+                    <Badge badgeType="danger" className="mr-2">
                       {intl.formatMessage(globalMessages.advanced)}
                     </Badge>
+                    <Badge badgeType="primary">
+                      {intl.formatMessage(globalMessages.restartRequired)}
+                    </Badge>
                     <span className="label-tip">
                       {intl.formatMessage(messages.csrfProtectionTip)}
                     </span>

src/components/Settings/SettingsServices.tsx

@@ -43,6 +43,7 @@ const messages = defineMessages({
     'A 4K {serverType} server must be marked as default in order to enable users to submit 4K {mediaType} requests.',
   mediaTypeMovie: 'movie',
   mediaTypeSeries: 'series',
+  deleteServer: 'Delete {serverType} Server',
 });
 
 interface ServerInstanceProps {
@@ -256,7 +257,7 @@ const SettingsServices: React.FC = () => {
         leaveTo="opacity-0"
       >
         <Modal
-          okText="Delete"
+          okText={intl.formatMessage(globalMessages.delete)}
           okButtonType="danger"
           onOk={() => deleteServer()}
           onCancel={() =>
@@ -266,7 +267,10 @@ const SettingsServices: React.FC = () => {
               type: 'radarr',
             })
           }
-          title="Delete Server"
+          title={intl.formatMessage(messages.deleteServer, {
+            serverType:
+              deleteServerModal.type === 'radarr' ? 'Radarr' : 'Sonarr',
+          })}
           iconSvg={<TrashIcon />}
         >
           {intl.formatMessage(messages.deleteserverconfirm)}