Merge pull request #7721 from opf/fix/31148/correctly-refresh-attachments

[31148] Correctly refresh attachments in attachment-list

frontend/src/app/components/work-packages/wp-single-view/wp-single-view.component.ts

@@ -156,10 +156,6 @@ export class WorkPackageSingleViewComponent implements OnInit, OnDestroy {
   public ngOnInit() {
     this.$element = jQuery(this.elementRef.nativeElement);
 
-    if (this.workPackage.attachments) {
-      this.workPackage.attachments.updateElements();
-    }
-
     // Whenever the resource context changes in any way,
     // update the visible fields.
     this.resourceContextChange

frontend/src/app/modules/attachments/attachment-list/attachment-list.component.ts

@@ -59,7 +59,7 @@ export class AttachmentListComponent implements OnInit, OnChanges, OnDestroy {
     this.$element = jQuery(this.elementRef.nativeElement);
 
     if (this.attachmentsUpdatable) {
-      this.resource.attachments.updateElements();
+      this.resource.updateAttachments();
     }
 
     this.attachments = this.resource.attachments.elements;
@@ -67,7 +67,8 @@ export class AttachmentListComponent implements OnInit, OnChanges, OnDestroy {
   }
 
   public setupResourceUpdateListener() {
-    this.states.forResource(this.resource)!.changes$()
+    this.states.forResource(this.resource)!
+      .values$()
       .pipe(
         takeUntil(componentDestroyed(this)),
         filter(newResource => !!newResource)

frontend/src/app/modules/hal/resources/collection-resource.ts

@@ -39,7 +39,7 @@ export class CollectionResource<T = HalResource> extends HalResource {
    * Update the collection's elements and return them in a promise.
    * This is useful, as angular does not recognize update made by $load.
    */
-  public updateElements() {
+  public updateElements():Promise<unknown> {
     if (this.$href) {
       return this.$load().then((collection:this) => this.elements = collection.elements);
     } else {

frontend/src/app/modules/hal/resources/mixins/attachable-mixin.ts

@@ -33,6 +33,7 @@ import {WorkPackageNotificationService} from 'core-components/wp-edit/wp-notific
 import {PathHelperService} from 'core-app/modules/common/path-helper/path-helper.service';
 import {NotificationsService} from 'core-app/modules/common/notifications/notifications.service';
 import {HttpErrorResponse} from "@angular/common/http";
+import {WorkPackageCacheService} from "core-components/work-packages/work-package-cache.service";
 
 type Constructor<T = {}> = new (...args:any[]) => T;
 
@@ -108,6 +109,22 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
       return Promise.resolve();
     }
 
+    /**
+     * Get updated attachments and activities from the server and push the state
+     *
+     * Return a promise that returns the attachments. Reject, if the work package has
+     * no attachments.
+     */
+    public updateAttachments():Promise<HalResource> {
+      return this
+        .attachments
+        .updateElements()
+        .then(() => {
+          this.updateState();
+          return this.attachments;
+        });
+    }
+
     /**
      * Upload the given attachments, update the resource and notify the user.
      * Return an updated AttachmentCollectionResource.
@@ -122,14 +139,11 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
         .then((result:{response:HalResource, uploadUrl:string }[]) => {
           setTimeout(() => this.NotificationsService.remove(notification), 700);
 
-          if (!!this.attachmentsBackend && !this.isNew) {
-            this.updateAttachments();
-          } else {
-            this.attachments.count += result.length;
-            result.forEach(r => {
-              this.attachments.elements.push(r.response);
-            });
-          }
+          this.attachments.count += result.length;
+          result.forEach(r => {
+            this.attachments.elements.push(r.response);
+          });
+          this.updateState();
 
           return result;
         })
@@ -164,6 +178,12 @@ export function Attachable<TBase extends Constructor<HalResource>>(Base:TBase) {
       return this.opFileUpload.uploadAndMapResponse(href, files);
     }
 
+    private updateState() {
+      if (this.state) {
+        this.state.putValue(this as any);
+      }
+    }
+
     public $initialize(source:any) {
       this.NotificationsService = this.injector.get(NotificationsService);
       this.wpNotificationsService = this.injector.get( WorkPackageNotificationService);

frontend/src/app/modules/hal/resources/work-package-resource.ts

@@ -46,6 +46,7 @@ import {NotificationsService} from 'core-app/modules/common/notifications/notifi
 import {Attachable} from 'core-app/modules/hal/resources/mixins/attachable-mixin';
 import {WorkPackageDmService} from "core-app/modules/hal/dm-services/work-package-dm.service";
 import {FormResource} from "core-app/modules/hal/resources/form-resource";
+import {InputState} from "reactivestates";
 
 export interface WorkPackageResourceEmbedded {
   activities:CollectionResource;
@@ -254,22 +255,6 @@ export class WorkPackageBaseResource extends HalResource {
     return promise;
   }
 
-  /**
-   * Get updated attachments and activities from the server and inform the cache service
-   * about the update.
-   *
-   * Return a promise that returns the attachments. Reject, if the work package has
-   * no attachments.
-   */
-  public updateAttachments():Promise<HalResource> {
-    return this
-      .updateLinkedResources('activities', 'attachments')
-      .then((resource:any) => {
-        this.wpCacheService.updateWorkPackage(this as any);
-        return resource.attachments;
-      });
-  }
-
   /**
    * Assign values from the form for a newly created work package resource.
    * @param form
@@ -346,6 +331,13 @@ export class WorkPackageBaseResource extends HalResource {
     return state.value!;
   }
 
+  /**
+   * Return the associated state to this HAL resource, if any.
+   */
+  public get state():InputState<this> {
+    return this.states.workPackages.get(this.id!) as any;
+  }
+
   public get hasOverriddenSchema():boolean {
     return this.overriddenSchema != null;
   }

spec/features/work_packages/navigation_spec.rb

@@ -210,4 +210,22 @@ RSpec.feature 'Work package navigation', js: true, selenium: true do
       wp_display.expect_state 'Table'
     end
   end
+
+  context 'work package with an attachment' do
+    let!(:attachment) { FactoryBot.build(:attachment, filename: 'attachment-first.pdf') }
+    let!(:wp_with_attachment) do
+      FactoryBot.create :work_package, subject: 'WP attachment A', project: project, attachments: [attachment]
+    end
+
+    it 'will show it when navigating from table to single view' do
+      wp_table = Pages::WorkPackagesTable.new project
+      wp_table.visit!
+
+      wp_table.expect_work_package_listed wp_with_attachment
+      full_view = wp_table.open_full_screen_by_link wp_with_attachment
+
+      full_view.ensure_page_loaded
+      expect(page).to have_selector('.work-package--attachments--filename', text: 'attachment-first.pdf', wait: 10)
+    end
+  end
 end