From 973ceb3b4b4a306caa9e8ecd8ad440e1036d9ebb Mon Sep 17 00:00:00 2001
From: Giuseppe Digilio <giuseppe.digilio@4science.it>
Date: Mon, 24 Jan 2022 19:54:21 +0100
Subject: [PATCH] [CST-4506] Address feedback

---
 ...onfig-accesses-conditions-options.model.ts |  5 +-
 .../models/config-submission-access.model.ts  | 20 ++++-
 .../models/access-condition.model.ts          | 25 ++++++
 .../models/submission-accesses.model.ts       | 37 --------
 .../submission-accesses.resource-type.ts      |  2 +-
 .../submission-item-access-condition.model.ts |  8 ++
 ...sion-upload-file-access-condition.model.ts | 23 +----
 .../workspaceitem-section-accesses.model.ts   | 27 +++---
 ...amic-form-control-container.component.html |  3 +-
 .../accesses/section-accesses.component.ts    | 90 +++++--------------
 .../accesses/section-accesses.model.ts        | 21 +++--
 .../accesses/section-accesses.service.ts      | 12 +--
 src/assets/i18n/en.json5                      |  2 +
 13 files changed, 115 insertions(+), 160 deletions(-)
 create mode 100644 src/app/core/submission/models/access-condition.model.ts
 delete mode 100644 src/app/core/submission/models/submission-accesses.model.ts
 create mode 100644 src/app/core/submission/models/submission-item-access-condition.model.ts

diff --git a/src/app/core/config/models/config-accesses-conditions-options.model.ts b/src/app/core/config/models/config-accesses-conditions-options.model.ts
index a12ae76b8e..244b501908 100644
--- a/src/app/core/config/models/config-accesses-conditions-options.model.ts
+++ b/src/app/core/config/models/config-accesses-conditions-options.model.ts
@@ -1,7 +1,5 @@
-import { autoserialize } from 'cerialize';
-
 /**
- * Model class for an Access Condition
+ * Model class for an Item Access Condition
  */
 export class AccessesConditionOption {
 
@@ -15,7 +13,6 @@ export class AccessesConditionOption {
      */
     groupName: string;
 
-
     /**
      * A boolean representing if this Access Condition has a start date
      */
diff --git a/src/app/core/config/models/config-submission-access.model.ts b/src/app/core/config/models/config-submission-access.model.ts
index 9584fc6e3c..7db96acf2b 100644
--- a/src/app/core/config/models/config-submission-access.model.ts
+++ b/src/app/core/config/models/config-submission-access.model.ts
@@ -1,27 +1,39 @@
-import { autoserialize, inheritSerialization, deserialize } from 'cerialize';
-import { typedObject, link } from '../../cache/builders/build-decorators';
+import { autoserialize, deserialize, inheritSerialization } from 'cerialize';
+import { typedObject } from '../../cache/builders/build-decorators';
 import { ConfigObject } from './config.model';
 import { AccessesConditionOption } from './config-accesses-conditions-options.model';
 import { SUBMISSION_ACCESSES_TYPE } from './config-type';
 import { HALLink } from '../../shared/hal-link.model';
 
-
+/**
+ * Class for the configuration describing the item accesses condition
+ */
 @typedObject
 @inheritSerialization(ConfigObject)
 export class SubmissionAccessModel extends ConfigObject {
   static type = SUBMISSION_ACCESSES_TYPE;
+
   /**
-   * A list of available bitstream access conditions
+   * A list of available item access conditions
    */
   @autoserialize
   accessConditionOptions: AccessesConditionOption[];
 
+  /**
+   * Boolean that indicates whether the current item must be findable via search or browse.
+   */
   @autoserialize
   discoverable: boolean;
 
+  /**
+   * Boolean that indicates whether or not the user can change the discoverable flag.
+   */
   @autoserialize
   canChangeDiscoverable: boolean;
 
+  /**
+   * The links to all related resources returned by the rest api.
+   */
   @deserialize
   _links: {
     self: HALLink
diff --git a/src/app/core/submission/models/access-condition.model.ts b/src/app/core/submission/models/access-condition.model.ts
new file mode 100644
index 0000000000..5ec5c2c434
--- /dev/null
+++ b/src/app/core/submission/models/access-condition.model.ts
@@ -0,0 +1,25 @@
+/**
+ * An interface to represent an access condition.
+ */
+export class AccessConditionObject {
+
+  /**
+   * The access condition id
+   */
+  id: string;
+
+  /**
+   * The access condition name
+   */
+  name: string;
+
+  /**
+   * Possible start date of the access condition
+   */
+  startDate: string;
+
+  /**
+   * Possible end date of the access condition
+   */
+  endDate: string;
+}
diff --git a/src/app/core/submission/models/submission-accesses.model.ts b/src/app/core/submission/models/submission-accesses.model.ts
deleted file mode 100644
index afad90b2ff..0000000000
--- a/src/app/core/submission/models/submission-accesses.model.ts
+++ /dev/null
@@ -1,37 +0,0 @@
-import { autoserialize, inheritSerialization } from 'cerialize';
-import { typedObject } from '../../cache/builders/build-decorators';
-import { excludeFromEquals } from '../../utilities/equals.decorators';
-import { ResourceType } from '../../shared/resource-type';
-import { HALResource } from '../../shared/hal-resource.model';
-import { SUBMISSION_ACCESSES } from './submission-accesses.resource-type';
-
-@typedObject
-@inheritSerialization(HALResource)
-export class SubmissionAccesses extends HALResource {
-
-    static type = SUBMISSION_ACCESSES;
-
-    /**
-     * The object type
-     */
-    @excludeFromEquals
-    @autoserialize
-    type: ResourceType;
-
-    @autoserialize
-    discoverable: boolean;
-
-    @autoserialize
-    accessConditions: AccessConditions[];
-}
-
-export interface AccessConditions {
-    name: string;
-    startDate?: Date;
-    hasStartDate?: boolean;
-    maxStartDate?: string;
-    hasEndDate?: boolean;
-    maxEndDate?: string;
-    endDate?: Date;
-}
-
diff --git a/src/app/core/submission/models/submission-accesses.resource-type.ts b/src/app/core/submission/models/submission-accesses.resource-type.ts
index 0aacb89522..0634a63337 100644
--- a/src/app/core/submission/models/submission-accesses.resource-type.ts
+++ b/src/app/core/submission/models/submission-accesses.resource-type.ts
@@ -1,7 +1,7 @@
 import { ResourceType } from '../../shared/resource-type';
 
 /**
- * The resource type for License
+ * The resource type for Accesses section
  *
  * Needs to be in a separate file to prevent circular
  * dependencies in webpack.
diff --git a/src/app/core/submission/models/submission-item-access-condition.model.ts b/src/app/core/submission/models/submission-item-access-condition.model.ts
new file mode 100644
index 0000000000..60e749b786
--- /dev/null
+++ b/src/app/core/submission/models/submission-item-access-condition.model.ts
@@ -0,0 +1,8 @@
+import { AccessConditionObject } from './access-condition.model';
+
+/**
+ * An interface to represent item's access condition.
+ */
+export class SubmissionItemAccessConditionObject extends AccessConditionObject {
+
+}
diff --git a/src/app/core/submission/models/submission-upload-file-access-condition.model.ts b/src/app/core/submission/models/submission-upload-file-access-condition.model.ts
index fa4d9b9062..7be9d6f1f7 100644
--- a/src/app/core/submission/models/submission-upload-file-access-condition.model.ts
+++ b/src/app/core/submission/models/submission-upload-file-access-condition.model.ts
@@ -1,25 +1,8 @@
+import { AccessConditionObject } from './access-condition.model';
+
 /**
  * An interface to represent bitstream's access condition.
  */
-export class SubmissionUploadFileAccessConditionObject {
+export class SubmissionUploadFileAccessConditionObject extends AccessConditionObject {
 
-  /**
-   * The access condition id
-   */
-  id: string;
-
-  /**
-   * The access condition name
-   */
-  name: string;
-
-  /**
-   * Possible start date of the access condition
-   */
-  startDate: string;
-
-  /**
-   * Possible end date of the access condition
-   */
-  endDate: string;
 }
diff --git a/src/app/core/submission/models/workspaceitem-section-accesses.model.ts b/src/app/core/submission/models/workspaceitem-section-accesses.model.ts
index c464f858cf..2d571f67f8 100644
--- a/src/app/core/submission/models/workspaceitem-section-accesses.model.ts
+++ b/src/app/core/submission/models/workspaceitem-section-accesses.model.ts
@@ -1,18 +1,21 @@
+import { SubmissionItemAccessConditionObject } from './submission-item-access-condition.model';
+
 /**
- * An interface to represent the submission's creative commons license section data.
+ * An interface to represent the submission's item accesses condition.
  */
 export interface WorkspaceitemSectionAccessesObject {
+  /**
+   * The access condition id
+   */
   id: string;
+
+  /**
+   * Boolean that indicates whether the current item must be findable via search or browse.
+   */
   discoverable: boolean;
-  accessConditions: [
-    {
-      name: string;
-      startDate?: Date;
-      hasStartDate?: boolean;
-      maxStartDate?: string;
-      hasEndDate?: boolean;
-      maxEndDate?: string;
-      endDate?: Date;
-    }
-  ];
+
+  /**
+   * A list of available item access conditions
+   */
+  accessConditions: SubmissionItemAccessConditionObject[];
 }
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html
index fc115e043a..55e354ea7a 100644
--- a/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html
+++ b/src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html
@@ -14,7 +14,8 @@
       <div>
         <ng-container #componentViewContainer></ng-container>
       </div>
-      <small *ngIf="hasHint && ((model.repeatable === false && (isRelationship === false || value?.value === null)) || (model.repeatable === true && context?.index === context?.context?.groups?.length - 1)) && (!showErrorMessages || errorMessages.length === 0)"
+
+      <small *ngIf="hasHint && ((!model.repeatable && (isRelationship === false || value?.value === null)) || (model.repeatable === true && context?.index === context?.context?.groups?.length - 1)) && (!showErrorMessages || errorMessages.length === 0)"
              class="text-muted ds-hint" [innerHTML]="model.hint | translate" [ngClass]="getClass('element', 'hint')"></small>
       <!-- In case of repeatable fields show empty space for all elements except the first -->
       <div *ngIf="context?.index !== null
diff --git a/src/app/submission/sections/accesses/section-accesses.component.ts b/src/app/submission/sections/accesses/section-accesses.component.ts
index 7f738e65a5..147e96a21f 100644
--- a/src/app/submission/sections/accesses/section-accesses.component.ts
+++ b/src/app/submission/sections/accesses/section-accesses.component.ts
@@ -1,11 +1,15 @@
 import { SectionAccessesService } from './section-accesses.service';
 import { Component, Inject, ViewChild } from '@angular/core';
+
+import { filter, map, mergeMap, take } from 'rxjs/operators';
+import { combineLatest, Observable, of, Subscription } from 'rxjs';
+import { TranslateService } from '@ngx-translate/core';
+
 import { renderSectionFor } from '../sections-decorator';
 import { SectionsType } from '../sections-type';
 import { SectionDataObject } from '../models/section-data.model';
 import { SectionsService } from '../sections.service';
 import { SectionModelComponent } from '../models/section.model';
-import { BehaviorSubject, combineLatest, Observable, of, Subscription } from 'rxjs';
 import {
   DYNAMIC_FORM_CONTROL_TYPE_CHECKBOX,
   DYNAMIC_FORM_CONTROL_TYPE_DATEPICKER,
@@ -19,13 +23,15 @@ import {
   MATCH_ENABLED,
   OR_OPERATOR
 } from '@ng-dynamic-forms/core';
-import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
 
+import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
 import {
   ACCESS_CONDITION_GROUP_CONFIG,
   ACCESS_CONDITION_GROUP_LAYOUT,
   ACCESS_CONDITIONS_FORM_ARRAY_CONFIG,
   ACCESS_CONDITIONS_FORM_ARRAY_LAYOUT,
+  ACCESS_FORM_CHECKBOX_CONFIG,
+  ACCESS_FORM_CHECKBOX_LAYOUT,
   FORM_ACCESS_CONDITION_END_DATE_CONFIG,
   FORM_ACCESS_CONDITION_END_DATE_LAYOUT,
   FORM_ACCESS_CONDITION_START_DATE_CONFIG,
@@ -37,17 +43,18 @@ import { hasValue, isNotEmpty, isNotNull } from '../../../shared/empty.util';
 import { WorkspaceitemSectionAccessesObject } from '../../../core/submission/models/workspaceitem-section-accesses.model';
 import { SubmissionAccessesConfigService } from '../../../core/config/submission-accesses-config.service';
 import { getFirstSucceededRemoteData } from '../../../core/shared/operators';
-import { filter, map, mergeMap, take } from 'rxjs/operators';
 import { FormComponent } from '../../../shared/form/form.component';
 import { FormService } from '../../../shared/form/form.service';
 import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder/json-patch-operation-path-combiner';
 import { SectionFormOperationsService } from '../form/section-form-operations.service';
 import { JsonPatchOperationsBuilder } from '../../../core/json-patch/builder/json-patch-operations-builder';
 import { AccessesConditionOption } from '../../../core/config/models/config-accesses-conditions-options.model';
-import { TranslateService } from '@ngx-translate/core';
 import { SubmissionJsonPatchOperationsService } from '../../../core/submission/submission-json-patch-operations.service';
 import { dateToISOFormat } from '../../../shared/date.util';
 
+/**
+ * This component represents a section for managing item's access conditions.
+ */
 @Component({
   selector: 'ds-section-accesses',
   templateUrl: './section-accesses.component.html',
@@ -62,7 +69,7 @@ export class SubmissionSectionAccessesComponent extends SectionModelComponent {
   @ViewChild('formRef') public formRef: FormComponent;
 
   /**
-   * List of available access conditions that could be set to files
+   * List of available access conditions that could be set to item
    */
   public availableAccessConditionOptions: AccessesConditionOption[];  // List of accessConditions that an user can select
 
@@ -73,52 +80,34 @@ export class SubmissionSectionAccessesComponent extends SectionModelComponent {
   public formId: string;
 
   /**
-   * The accesses metadata data
+   * The accesses section data
    * @type {WorkspaceitemSectionAccessesObject}
    */
   public accessesData: WorkspaceitemSectionAccessesObject;
-  /**
-   * The collection name this submission belonging to
-   * @type {string}
-   */
-  public collectionName: string;
-  /**
-   * Is the upload required
-   * @type {boolean}
-   */
-  public required$ = new BehaviorSubject<boolean>(true);
+
   /**
    * The form model
    * @type {DynamicFormControlModel[]}
    */
-  formModel: DynamicFormControlModel[];
+  public formModel: DynamicFormControlModel[];
+
   /**
    * Array to track all subscriptions and unsubscribe them onDestroy
    * @type {Array}
    */
   protected subs: Subscription[] = [];
+
   /**
    * The [[JsonPatchOperationPathCombiner]] object
    * @type {JsonPatchOperationPathCombiner}
    */
   protected pathCombiner: JsonPatchOperationPathCombiner;
-  /**
-   * A map representing all field prevous values
-   * @type {Map}
-   */
-  protected previousValue: Map<string, number[]> = new Map();
-  /**
-   * A map representing all field on their way to be removed
-   * @type {Map}
-   */
-  protected fieldsOnTheirWayToBeRemoved: Map<string, number[]> = new Map();
 
   /**
    * Defines if the access discoverable property can be managed
    */
   public canChangeDiscoverable: boolean;
 
-
   /**
    * Initialize instance variables
    *
@@ -233,36 +222,6 @@ export class SubmissionSectionAccessesComponent extends SectionModelComponent {
     }
   }
 
-  /**
-   * Check if the specified form field has already a value stored
-   *
-   * @param fieldId
-   *    the section data retrieved from the serverù
-   * @param index
-   *    the section data retrieved from the server
-   */
-  hasStoredValue(fieldId, index): boolean {
-    if (isNotEmpty(this.sectionData.data)) {
-      return this.sectionData.data.hasOwnProperty(fieldId) &&
-        isNotEmpty(this.sectionData.data[fieldId][index]) &&
-        !this.isFieldToRemove(fieldId, index);
-    } else {
-      return false;
-    }
-  }
-
-  /**
-   * Check if the specified field is on the way to be removed
-   *
-   * @param fieldId
-   *    the section data retrieved from the serverù
-   * @param index
-   *    the section data retrieved from the server
-   */
-  isFieldToRemove(fieldId, index) {
-    return this.fieldsOnTheirWayToBeRemoved.has(fieldId) && this.fieldsOnTheirWayToBeRemoved.get(fieldId).includes(index);
-  }
-
   /**
    * Unsubscribe from all subscriptions
    */
@@ -286,7 +245,7 @@ export class SubmissionSectionAccessesComponent extends SectionModelComponent {
 
     const accessData$ = this.accessesService.getAccessesData(this.submissionId, this.sectionData.id);
 
-    combineLatest(config$, accessData$).subscribe(([config, accessData]) => {
+    combineLatest([config$, accessData$]).subscribe(([config, accessData]) => {
       this.availableAccessConditionOptions = isNotEmpty(config.accessConditionOptions) ? config.accessConditionOptions : [];
       this.canChangeDiscoverable = !!config.canChangeDiscoverable;
       this.accessesData = accessData;
@@ -313,17 +272,16 @@ export class SubmissionSectionAccessesComponent extends SectionModelComponent {
 
     const formModel: DynamicFormControlModel[] = [];
     if (this.canChangeDiscoverable) {
+      const discoverableCheckboxConfig = Object.assign({}, ACCESS_FORM_CHECKBOX_CONFIG, {
+        label: this.translate.instant('submission.sections.accesses.form.discoverable-label'),
+        hint: this.translate.instant('submission.sections.accesses.form.discoverable-description'),
+        value: this.accessesData.discoverable
+      });
       formModel.push(
-        new DynamicCheckboxModel({
-          id: 'discoverable',
-          label: this.translate.instant('submission.sections.accesses.form.discoverable-label'),
-          name: 'discoverable',
-          value: this.accessesData.discoverable
-        })
+        new DynamicCheckboxModel(discoverableCheckboxConfig, ACCESS_FORM_CHECKBOX_LAYOUT)
       );
     }
 
-
     const accessConditionTypeModelConfig = Object.assign({}, FORM_ACCESS_CONDITION_TYPE_CONFIG);
     const accessConditionsArrayConfig = Object.assign({}, ACCESS_CONDITIONS_FORM_ARRAY_CONFIG);
     const accessConditionTypeOptions = [];
diff --git a/src/app/submission/sections/accesses/section-accesses.model.ts b/src/app/submission/sections/accesses/section-accesses.model.ts
index 6b8178f9d5..434ee04741 100644
--- a/src/app/submission/sections/accesses/section-accesses.model.ts
+++ b/src/app/submission/sections/accesses/section-accesses.model.ts
@@ -7,19 +7,22 @@ import {
   MATCH_ENABLED,
   OR_OPERATOR,
 } from '@ng-dynamic-forms/core';
+import { DynamicCheckboxModelConfig } from '@ng-dynamic-forms/core/lib/model/checkbox/dynamic-checkbox.model';
 
 
-export const ACCESS_FORM_CHECKBOX_LAYOUT: DynamicFormControlLayout = {
-  element: {
-    host: 'form-group flex-fill access-condition-group',
-    id: 'discoverable',
-    // disabled: false,
-    label: 'submission.sections.accesses.form.discoverable-label',
-    name: 'discoverable',
-  }
+export const ACCESS_FORM_CHECKBOX_CONFIG: DynamicCheckboxModelConfig = {
+  id: 'discoverable',
+  name: 'discoverable'
 };
 
+export const ACCESS_FORM_CHECKBOX_LAYOUT = {
 
+  element: {
+    container: 'custom-control custom-checkbox pl-1',
+    control: 'custom-control-input',
+    label: 'custom-control-label pt-1'
+  }
+};
 
 export const ACCESS_CONDITION_GROUP_CONFIG: DynamicFormGroupModelConfig = {
   id: 'accessConditionGroup',
@@ -28,7 +31,7 @@ export const ACCESS_CONDITION_GROUP_CONFIG: DynamicFormGroupModelConfig = {
 
 export const ACCESS_CONDITION_GROUP_LAYOUT: DynamicFormControlLayout = {
   element: {
-    host: 'form-group flex-fill access-condition-group',
+    host: 'form-group flex-fill',
     container: 'pl-1 pr-1',
     control: 'form-row '
   }
diff --git a/src/app/submission/sections/accesses/section-accesses.service.ts b/src/app/submission/sections/accesses/section-accesses.service.ts
index 3845f71871..f358582136 100644
--- a/src/app/submission/sections/accesses/section-accesses.service.ts
+++ b/src/app/submission/sections/accesses/section-accesses.service.ts
@@ -5,12 +5,12 @@ import { distinctUntilChanged, filter } from 'rxjs/operators';
 import { Store } from '@ngrx/store';
 
 import { SubmissionState } from '../../submission.reducers';
-import { isUndefined } from 'util';
+import { isNotUndefined } from '../../../shared/empty.util';
 import { submissionSectionDataFromIdSelector } from '../../selectors';
-
+import { WorkspaceitemSectionAccessesObject } from '../../../core/submission/models/workspaceitem-section-accesses.model';
 
 /**
- * A service that provides methods to handle submission's bitstream state.
+ * A service that provides methods to handle submission item's accesses condition state.
  */
 @Injectable()
 export class SectionAccessesService {
@@ -24,7 +24,7 @@ export class SectionAccessesService {
 
 
   /**
-   * Return bitstream's metadata
+   * Return item's accesses condition state.
    *
    * @param submissionId
    *    The submission id
@@ -33,10 +33,10 @@ export class SectionAccessesService {
    * @returns {Observable}
    *    Emits bitstream's metadata
    */
-  public getAccessesData(submissionId: string, sectionId: string): Observable<any> {
+  public getAccessesData(submissionId: string, sectionId: string): Observable<WorkspaceitemSectionAccessesObject> {
 
     return this.store.select(submissionSectionDataFromIdSelector(submissionId, sectionId)).pipe(
-      filter((state) => !isUndefined(state)),
+      filter((state) => isNotUndefined(state)),
       distinctUntilChanged());
   }
 }
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index e1c0fed2ca..6892685f8d 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -3895,6 +3895,8 @@
 
   "submission.sections.upload.upload-successful": "Upload successful",
 
+  "submission.sections.accesses.form.discoverable-description": "When checked, this item will be discoverable in search/browse. When unchecked, the item will only be available via a direct link and will never appear in search/browse.",
+
   "submission.sections.accesses.form.discoverable-label": "Discoverable",