]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-19437 Browser fails to display previous page
authorWouter Admiraal <wouter.admiraal@sonarsource.com>
Tue, 13 Jun 2023 10:47:04 +0000 (12:47 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 13 Jun 2023 20:03:38 +0000 (20:03 +0000)
server/sonar-web/src/main/js/app/components/ProjectAdminContainer.tsx
server/sonar-web/src/main/js/app/components/__tests__/ProjectAdminContainer-test.tsx

index 5d1ded1dc5b08ad30c8b353e928f600195dbfdc2..25eeb223b684f324e8b21f8aabbe7e9f13788ddd 100644 (file)
@@ -29,24 +29,47 @@ interface Props {
 }
 
 export class ProjectAdminContainer extends React.PureComponent<Props> {
+  mounted = false;
+
   componentDidMount() {
-    this.checkPermissions();
+    // We need to check permissions *after* the parent ComponentContainer is updated.
+    // This is why we wrap checkPermission() in a setTimeout().
+    //
+    // We want to prevent an edge-case where if you navigate from an admin component page
+    // to another component page where you have "read" access but no admin access, and
+    // then hit the browser's Back button, you will get redirected to the login page.
+    //
+    // The reason is that this component will get mounted *before* the parent
+    // ComponentContainer is *updated*. The parent component still has the last component
+    // stored in state, the one where the user might not have admin access. It will detect
+    // the change in URL and trigger a refresh of its state, but this comes too late; the
+    // checkPermission() here will already have triggered (because mounts happen before updates)
+    // and redirected the user. Wrapping it in a setTimeout() allows the parent component
+    // to refresh, unmounting this component, and only triggering the redirect if it makes sense.
+    //
+    // See https://sonarsource.atlassian.net/browse/SONAR-19437
+    setTimeout(this.checkPermissions, 0);
+    this.mounted = true;
   }
 
   componentDidUpdate() {
     this.checkPermissions();
   }
 
-  checkPermissions() {
-    if (!this.isProjectAdmin()) {
+  componentWillUnmount() {
+    this.mounted = false;
+  }
+
+  checkPermissions = () => {
+    if (this.mounted && !this.isProjectAdmin()) {
       handleRequiredAuthorization();
     }
-  }
+  };
 
-  isProjectAdmin() {
+  isProjectAdmin = () => {
     const { configuration } = this.props.component;
     return configuration != null && configuration.showSettings;
-  }
+  };
 
   render() {
     if (!this.isProjectAdmin()) {
index 75fdefb7f3c4429046d09f482e5cd4e935e330d4..d3ed20ab64f710e87aef30f3a0f9a9e354bee19f 100644 (file)
@@ -32,8 +32,11 @@ it('should render correctly', () => {
 });
 
 it('should redirect for authorization if needed', () => {
+  jest.useFakeTimers();
   mountRender({ component: mockComponent({ configuration: { showSettings: false } }) });
+  jest.runAllTimers();
   expect(handleRequiredAuthorization).toHaveBeenCalled();
+  jest.useRealTimers();
 });
 
 function mountRender(props: Partial<ProjectAdminContainer['props']> = {}) {