Sonarcloud issues fixes (#3499)

* Sonarcloud issues fixes

* Code smell fixes

* Refactoring to remove new code duplications

* Sonarcloud code smell fixes part I

* Sonarcloud code smell fixes part II

* Missing code smell fix

* Add new ESLint rules to cover fixed SonarCloud issues

* Add missing command

* Add missing is existing check

projects/aca-shared/rules/src/app.rules.ts

@@ -178,7 +178,7 @@ export function isShared(context: RuleContext): boolean {
   }
 
   if (navigation.isNotTrashcan(context) && !context.selection.isEmpty && context.selection.file) {
-    return !!(context.selection.file.entry && context.selection.file.entry.properties && context.selection.file.entry.properties['qshare:sharedId']);
+    return !!context.selection.file.entry?.properties?.['qshare:sharedId'];
   }
 
   return false;
@@ -268,14 +268,7 @@ export const canCreateLibrary = (context: AcaRuleContext): boolean =>
  * JSON ref: `app.navigation.folder.canUpload`
  */
 export function canUpload(context: AcaRuleContext): boolean {
-  if (isContentServiceEnabled(context) && (navigation.isPersonalFiles(context) || navigation.isLibraryContent(context))) {
-    const { currentFolder } = context.navigation;
-
-    if (currentFolder) {
-      return context.permissions.check(currentFolder, ['create']);
-    }
-  }
-  return false;
+  return canCreateFolder(context);
 }
 
 /**
@@ -306,8 +299,7 @@ export const hasLibrarySelected = (context: RuleContext): boolean => !!context.s
  * JSON ref: `app.selection.isPrivateLibrary`
  */
 export function isPrivateLibrary(context: RuleContext): boolean {
-  const library = context.selection.library;
-  return library ? !!(library.entry && library.entry.visibility && library.entry.visibility === 'PRIVATE') : false;
+  return context.selection.library?.entry?.visibility === 'PRIVATE';
 }
 
 /**
@@ -316,7 +308,7 @@ export function isPrivateLibrary(context: RuleContext): boolean {
  */
 export function hasLibraryRole(context: RuleContext): boolean {
   const library = context.selection.library;
-  return library ? !!(library.entry && library.entry.role) : false;
+  return library ? !!library.entry?.role : false;
 }
 
 /**
@@ -329,7 +321,7 @@ export const hasNoLibraryRole = (context: RuleContext): boolean => !hasLibraryRo
  * Checks if user has selected a file.
  * JSON ref: `app.selection.file`
  */
-export const hasFileSelected = (context: RuleContext): boolean => !!(context && context.selection && context.selection.file);
+export const hasFileSelected = (context: RuleContext): boolean => !!context?.selection?.file;
 
 /**
  * Checks if user can update the first selected node.
@@ -376,13 +368,13 @@ export function canUpdateSelectedFolder(context: RuleContext): boolean {
  * JSON ref: `app.selection.file.isLocked`
  */
 export function hasLockedFiles(context: RuleContext): boolean {
-  if (context && context.selection && context.selection.nodes) {
+  if (context?.selection?.nodes) {
     return context.selection.nodes.some((node) => {
       if (!node.entry.isFile) {
         return false;
       }
 
-      return node.entry.isLocked || (node.entry.properties && node.entry.properties['cm:lockType'] === 'READ_ONLY_LOCK');
+      return node.entry.isLocked || node.entry.properties?.['cm:lockType'] === 'READ_ONLY_LOCK';
     });
   }
 
@@ -395,13 +387,8 @@ export function hasLockedFiles(context: RuleContext): boolean {
  */
 export const isWriteLocked = (context: RuleContext): boolean =>
   !!(
-    context &&
-    context.selection &&
-    context.selection.file &&
-    context.selection.file.entry &&
-    context.selection.file.entry.properties &&
-    (context.selection.file.entry.properties['cm:lockType'] === 'WRITE_LOCK' ||
-      context.selection.file.entry.properties['cm:lockType'] === 'READ_ONLY_LOCK')
+    context?.selection?.file?.entry?.properties?.['cm:lockType'] === 'WRITE_LOCK' ||
+    context?.selection?.file?.entry?.properties?.['cm:lockType'] === 'READ_ONLY_LOCK'
   );
 
 /**
@@ -589,17 +576,17 @@ export function canOpenWithOffice(context: AcaRuleContext): boolean {
     return false;
   }
 
-  if (context.navigation && context.navigation.url && context.navigation.url.startsWith('/trashcan')) {
+  if (context.navigation?.url?.startsWith('/trashcan')) {
     return false;
   }
 
-  if (!context || !context.selection) {
+  if (!context?.selection) {
     return false;
   }
 
   const { file } = context.selection;
 
-  if (!file || !file.entry) {
+  if (!file?.entry) {
     return false;
   }
 
@@ -626,7 +613,7 @@ export function canOpenWithOffice(context: AcaRuleContext): boolean {
   }
 
   // workaround for Shared files
-  if (context.navigation && context.navigation.url && context.navigation.url.startsWith('/shared')) {
+  if (context.navigation?.url?.startsWith('/shared')) {
     // eslint-disable-next-line no-prototype-builtins
     if (file.entry.hasOwnProperty('allowableOperationsOnTarget')) {
       return context.permissions.check(file, ['update'], {

projects/aca-shared/rules/src/navigation.rules.ts

@@ -39,7 +39,7 @@ export function isPreview(context: RuleContext): boolean {
  */
 export function isFavorites(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/favorites') && !isPreview(context);
+  return url?.startsWith('/favorites') && !isPreview(context);
 }
 
 /**
@@ -54,7 +54,7 @@ export const isNotFavorites = (context: RuleContext): boolean => !isFavorites(co
  */
 export function isSharedFiles(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/shared') && !isPreview(context);
+  return url?.startsWith('/shared') && !isPreview(context);
 }
 
 /**
@@ -69,7 +69,7 @@ export const isNotSharedFiles = (context: RuleContext): boolean => !isSharedFile
  */
 export function isTrashcan(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/trashcan');
+  return url?.startsWith('/trashcan');
 }
 
 /**
@@ -84,7 +84,7 @@ export const isNotTrashcan = (context: RuleContext): boolean => !isTrashcan(cont
  */
 export function isPersonalFiles(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/personal-files');
+  return url?.startsWith('/personal-files');
 }
 
 /**
@@ -93,7 +93,7 @@ export function isPersonalFiles(context: RuleContext): boolean {
  */
 export function isLibraryFiles(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/libraries');
+  return url?.startsWith('/libraries');
 }
 
 /**
@@ -102,12 +102,12 @@ export function isLibraryFiles(context: RuleContext): boolean {
  */
 export function isLibraries(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && (url.endsWith('/libraries') || url.startsWith('/search-libraries'));
+  return url?.endsWith('/libraries') || url?.startsWith('/search-libraries');
 }
 
 export function isLibraryContent(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && (url.endsWith('/libraries') || url.includes('/libraries/') || url.startsWith('/search-libraries'));
+  return url?.endsWith('/libraries') || url?.includes('/libraries/') || url?.startsWith('/search-libraries');
 }
 
 /**
@@ -122,7 +122,7 @@ export const isNotLibraries = (context: RuleContext): boolean => !isLibraries(co
  */
 export function isRecentFiles(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/recent-files');
+  return url?.startsWith('/recent-files');
 }
 
 /**
@@ -140,7 +140,7 @@ export function isSearchResults(
   // ...args: RuleParameter[]
 ): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/search');
+  return url?.startsWith('/search');
 }
 
 /**
@@ -155,7 +155,7 @@ export const isNotSearchResults = (context: RuleContext): boolean => !isSearchRe
  */
 export function isSharedPreview(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && (url.startsWith('/shared/preview/') || (url.startsWith('/shared') && url.includes('viewer:view')));
+  return url?.startsWith('/shared/preview/') || (url?.startsWith('/shared') && url?.includes('viewer:view'));
 }
 
 /**
@@ -164,7 +164,7 @@ export function isSharedPreview(context: RuleContext): boolean {
  */
 export function isFavoritesPreview(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && (url.startsWith('/favorites/preview/') || (url.startsWith('/favorites') && url.includes('viewer:view')));
+  return url?.startsWith('/favorites/preview/') || (url?.startsWith('/favorites') && url?.includes('viewer:view'));
 }
 
 /**
@@ -173,5 +173,5 @@ export function isFavoritesPreview(context: RuleContext): boolean {
  */
 export function isSharedFileViewer(context: RuleContext): boolean {
   const { url } = context.navigation;
-  return url && url.startsWith('/preview/s/');
+  return url?.startsWith('/preview/s/');
 }

projects/aca-shared/src/lib/components/document-base-page/document-base-page.component.ts

@@ -130,7 +130,7 @@ export abstract class PageComponent implements OnInit, OnDestroy, OnChanges {
   }
 
   ngOnChanges(changes: SimpleChanges) {
-    if (changes.nodeResult && changes.nodeResult.currentValue) {
+    if (changes.nodeResult?.currentValue) {
       this.nodeResult = changes.nodeResult.currentValue;
     }
   }
@@ -145,7 +145,7 @@ export abstract class PageComponent implements OnInit, OnDestroy, OnChanges {
   }
 
   showPreview(node: NodeEntry, extras?: ViewNodeExtras) {
-    if (node && node.entry) {
+    if (node?.entry) {
       if (this.fileAutoDownloadService?.shouldFileAutoDownload(node.entry?.content?.sizeInBytes)) {
         this.fileAutoDownloadService.autoDownloadFile(node);
       } else {

projects/aca-shared/src/lib/components/document-base-page/document-base-page.spec.ts

@@ -91,10 +91,6 @@ class DocumentBasePageServiceMock extends DocumentBasePageService {
 class TestComponent extends PageComponent {
   node: any;
 
-  constructor() {
-    super();
-  }
-
   addSubscription(entry: Subscription) {
     this.subscriptions.push(entry);
   }

projects/aca-shared/src/lib/components/toolbar/toolbar-button/toolbar-button.component.ts

@@ -74,6 +74,6 @@ export class ToolbarButtonComponent {
   }
 
   private hasClickAction(actionRef: ContentActionRef): boolean {
-    return !!(actionRef && actionRef.actions && actionRef.actions.click);
+    return !!actionRef?.actions?.click;
   }
 }

projects/aca-shared/src/lib/components/toolbar/toolbar-menu-item/toolbar-menu-item.component.ts

@@ -71,7 +71,7 @@ export class ToolbarMenuItemComponent {
   }
 
   private hasClickAction(actionRef: ContentActionRef): boolean {
-    return !!(actionRef && actionRef.actions && actionRef.actions.click);
+    return !!actionRef?.actions?.click;
   }
 
   trackByActionId(_: number, obj: ContentActionRef): string {

projects/aca-shared/src/lib/directives/contextmenu/contextmenu.directive.ts

@@ -98,11 +98,9 @@ export class ContextActionsDirective implements OnInit, OnDestroy {
   }
 
   private findAncestor(el: Element, className: string): Element {
-    if (el.classList.contains(className)) {
-      return el;
+    while (el && !el.classList.contains(className)) {
+      el = el.parentElement;
     }
-    // eslint-disable-next-line curly
-    while ((el = el.parentElement) && !el.classList.contains(className));
     return el;
   }
 }

projects/aca-shared/src/lib/services/app.extension.service.ts

@@ -180,7 +180,7 @@ export class AppExtensionService implements RuleContext {
 
     this.withCredentials = this.appConfig.get<boolean>('auth.withCredentials', false);
 
-    if (config.features && config.features.viewer) {
+    if (config.features?.viewer) {
       this.viewerRules = (config.features.viewer['rules'] as ViewerRules) || {};
     }
 
@@ -340,7 +340,7 @@ export class AppExtensionService implements RuleContext {
   private setActionDisabledFromRule(action: ContentActionRef) {
     let disabled = false;
 
-    if (action && action.rules && action.rules.enabled) {
+    if (action?.rules?.enabled) {
       disabled = !this.extensions.evaluateRule(action.rules.enabled, this);
     }
 
@@ -483,7 +483,7 @@ export class AppExtensionService implements RuleContext {
   }
 
   filterVisible(action: ContentActionRef | SettingsGroupRef | SidebarTabRef | DocumentListPresetRef): boolean {
-    if (action && action.rules && action.rules.visible) {
+    if (action?.rules?.visible) {
       return this.extensions.evaluateRule(action.rules.visible, this);
     }
     return true;
@@ -495,7 +495,7 @@ export class AppExtensionService implements RuleContext {
         return true;
       }
 
-      if (extension.rules && extension.rules.disabled) {
+      if (extension.rules?.disabled) {
         return this.extensions.evaluateRule(extension.rules.disabled, this);
       }
     }

projects/aca-shared/store/src/actions/router.actions.ts

@@ -52,6 +52,4 @@ export class NavigateToParentFolder implements Action {
 
 export class NavigateToPreviousPage implements Action {
   readonly type = RouterActionTypes.NavigateToPreviousPage;
-
-  constructor() {}
 }

projects/aca-shared/store/src/actions/search.actions.ts

@@ -32,7 +32,6 @@ export enum SearchActionTypes {
 
 export class SearchAction implements Action {
   readonly type = SearchActionTypes.Search;
-  constructor() {}
 }
 
 export class SearchByTermAction implements Action {

projects/aca-shared/store/src/actions/template.actions.ts

@@ -28,14 +28,10 @@ import { TemplateActionTypes } from './template-action-types';
 
 export class FileFromTemplate implements Action {
   readonly type = TemplateActionTypes.FileFromTemplate;
-
-  constructor() {}
 }
 
 export class FolderFromTemplate implements Action {
   readonly type = TemplateActionTypes.FolderFromTemplate;
-
-  constructor() {}
 }
 
 export class CreateFromTemplate implements Action {

projects/aca-shared/store/src/effects/router.effects.ts

@@ -67,7 +67,7 @@ export class RouterEffects {
       this.actions$.pipe(
         ofType<NavigateToFolder>(RouterActionTypes.NavigateFolder),
         map((action) => {
-          if (action.payload && action.payload.entry) {
+          if (action.payload?.entry) {
             this.navigateToFolder(action.payload.entry);
           }
         })
@@ -80,7 +80,7 @@ export class RouterEffects {
       this.actions$.pipe(
         ofType<NavigateToParentFolder>(RouterActionTypes.NavigateParentFolder),
         map((action) => {
-          if (action.payload && action.payload.entry) {
+          if (action.payload?.entry) {
             this.navigateToParentFolder(action.payload.entry);
           }
         })
@@ -101,7 +101,7 @@ export class RouterEffects {
     let link: any[] = null;
     const { path, id } = node;
 
-    if (path && path.name && path.elements) {
+    if (path?.name && path?.elements) {
       const isLibraryPath = this.isLibraryContent(path);
 
       const parent = path.elements[path.elements.length - 1];
@@ -126,7 +126,7 @@ export class RouterEffects {
     let link: any[] = null;
     const { path } = node;
 
-    if (path && path.name && path.elements) {
+    if (path?.name && path?.elements) {
       const isLibraryPath = this.isLibraryContent(path);
 
       const parent = path.elements[path.elements.length - 1];