fix bug when modifying illustration on a challenge

Co-authored-by: Iris Benoît <[email protected]>
Co-authored-by: Fael Bassetti <[email protected]>

pix-editor/app/components/form/challenge.js

@@ -157,29 +157,31 @@ export default class ChallengeForm extends Component {
   }
 
   @action
-  addIllustration(file) {
-    const attachment = {
+  async addIllustration(file) {
+    const attachmentData = {
       filename: file.name,
       size: file.size,
       mimeType: file.type,
       file,
       type: 'illustration',
-      challenge: this.args.challenge,
     };
-    this.store.createRecord('attachment', attachment);
+    const attachment = this.store.createRecord('attachment', attachmentData);
+    const files = await this.args.challenge.files;
+    files.push(attachment);
   }
 
   @action
-  addAttachment(file) {
-    const attachment = {
+  async addAttachment(file) {
+    const attachmentData = {
       filename: file.name,
       size: file.size,
       mimeType: file.type,
       file,
       type: 'attachment',
-      challenge: this.args.challenge,
     };
-    this.store.createRecord('attachment', attachment);
+    const attachment = this.store.createRecord('attachment', attachmentData);
+    const files = await this.args.challenge.files;
+    files.push(attachment);
   }
 
   @action

pix-editor/app/controllers/authenticated/competence/prototypes/localized.js

@@ -190,7 +190,7 @@ export default class LocalizedController extends Controller {
     try {
       await this._handleIllustration(this.localizedChallenge);
       await this._handleAttachments(this.localizedChallenge);
-      await this._saveAttachments(this.localizedChallenge);
+      await this._saveFiles(this.localizedChallenge);
       await this._saveChallenge(this.localizedChallenge);
       this.edition = false;
       this.invalidUrlsToConsult = '';
@@ -239,18 +239,20 @@ export default class LocalizedController extends Controller {
   }
 
   @action
-  addIllustration(file, alt = '') {
-    const attachment = {
+  async addIllustration(file, alt = '') {
+    const attachmentData = {
       filename: file.name,
       size: file.size,
       mimeType: file.type,
       file,
       type: 'illustration',
-      localizedChallenge: this.localizedChallenge,
-      challenge: this.challenge,
       alt,
     };
-    this.store.createRecord('attachment', attachment);
+    const attachment = this.store.createRecord('attachment', attachmentData);
+    const files = await this.challenge.files;
+    const localizedFiles = await this.localizedChallenge.files;
+    files.push(attachment);
+    localizedFiles.push(attachment);
   }
 
   @action
@@ -259,34 +261,36 @@ export default class LocalizedController extends Controller {
     const removedFile = this.localizedChallenge.illustration;
     if (removedFile) {
       removedFile.deleteRecord();
-      if (removedFile.id) {
+      if (!removedFile.isNew) {
         this.deletedFiles.push(removedFile);
       }
       return removedFile.alt;
     }
   }
 
   @action
-  addAttachment(file) {
-    const attachment = {
+  async addAttachment(file) {
+    const attachmentData = {
       filename: file.name,
       size: file.size,
       mimeType: file.type,
       file,
       type: 'attachment',
-      localizedChallenge: this.localizedChallenge,
-      challenge: this.challenge,
     };
-    this.store.createRecord('attachment', attachment);
+    const attachment = this.store.createRecord('attachment', attachmentData);
+    const files = await this.challenge.files;
+    const localizedFiles = await this.localizedChallenge.files;
+    files.push(attachment);
+    localizedFiles.push(attachment);
   }
 
   @action
   async removeAttachment(removedAttachment) {
-    const files = this.localizedChallenge.hasMany('files').value() ?? [];
+    const files = await this.localizedChallenge.files;
     const removedFile = files.find((file) => file.filename === removedAttachment.filename);
     if (removedFile) {
       removedFile.deleteRecord();
-      if (removedFile.id) {
+      if (!removedFile.isNew) {
         this.deletedFiles.push(removedFile);
       }
     }
@@ -343,8 +347,8 @@ export default class LocalizedController extends Controller {
     return challenge.save();
   }
 
-  async _saveAttachments(challenge) {
-    const files = await challenge.files;
+  async _saveFiles(challenge) {
+    const files = (await challenge.files)?.slice() ?? [];
     for (const file of files) {
       await file.save();
     }

pix-editor/app/controllers/authenticated/competence/prototypes/new.js

@@ -33,7 +33,7 @@ export default class NewController extends Prototype {
       await this._handleAttachments(this.challenge);
       // create challenge without patching Pix API cache
       await this._saveChallenge(this.challenge);
-      await this._saveAttachments(this.challenge);
+      await this._saveFiles(this.challenge);
       await this._setVersion(this.challenge);
       // update challenge's version and patch Pix API cache
       await this._saveChallenge(this.challenge);

pix-editor/app/controllers/authenticated/competence/prototypes/single.js

@@ -221,7 +221,7 @@ export default class SingleController extends Controller {
     return Promise.resolve(this.challenge)
       .then((challenge) => this._handleIllustration(challenge))
       .then((challenge) => this._handleAttachments(challenge))
-      .then((challenge) => this._saveAttachments(challenge))
+      .then((challenge) => this._saveFiles(challenge))
       .then((challenge) => this._saveChallenge(challenge))
       .then((challenge) => this._handleChangelog(challenge, changelog))
       .then(() => {
@@ -680,8 +680,8 @@ export default class SingleController extends Controller {
     if (!challenge.baseNameUpdated()) {
       return;
     }
-    const attachments = await challenge.attachments;
-    for (const file of attachments.toArray()) {
+    const attachments = (await challenge.attachments)?.slice() ?? [];
+    for (const file of attachments) {
       file.filename = this._getAttachmentFullFilename(challenge, file.filename);
       await this.storage.renameFile(file.url, file.filename);
     }
@@ -697,8 +697,8 @@ export default class SingleController extends Controller {
     return challenge.save();
   }
 
-  async _saveAttachments(challenge) {
-    const files = await challenge.files;
+  async _saveFiles(challenge) {
+    const files = (await challenge.files)?.slice() ?? [];
     for (const file of files) {
       if (file.cloneBeforeSave) {
         file.url = await this.storage.cloneFile(file.url);

pix-editor/app/controllers/authenticated/competence/prototypes/single/alternatives/new.js

@@ -32,7 +32,7 @@ export default class NewController extends Alternative {
       await this._handleAttachments(this.challenge);
       // create challenge without patching Pix API cache
       await this._saveChallenge(this.challenge);
-      await this._saveAttachments(this.challenge);
+      await this._saveFiles(this.challenge);
       await this._setAlternativeVersion(this.challenge);
       // update challenge's alternative version and patch Pix API cache
       await this._saveChallenge(this.challenge);
@@ -51,7 +51,7 @@ export default class NewController extends Alternative {
 
   async _setAlternativeVersion(challenge) {
     const skill = challenge.skill;
-    skill.reload();
+    await skill.reload();
     const version = await this.currentData.getPrototype().getNextAlternativeVersion();
     challenge.alternativeVersion = version;
   }

pix-editor/app/models/challenge.js

@@ -207,6 +207,11 @@ export default class ChallengeModel extends Model {
     return this._firstAttachmentBaseName;
   }
 
+  set attachmentBaseName(value) {
+    this._definedBaseName = value;
+    return value;
+  }
+
   get _firstAttachmentBaseName() {
     const attachments = this.attachments;
     if (attachments && attachments.length > 0) {
@@ -224,11 +229,6 @@ export default class ChallengeModel extends Model {
     return localizedChallenges.filter((localizedChallenge) => localizedChallenge.locale !== this.primaryLocale);
   }
 
-  set attachmentBaseName(value) {
-    this._definedBaseName = value;
-    return value;
-  }
-
   archive() {
     this.status = 'archivé';
     this.archivedAt = new Date();
@@ -323,7 +323,7 @@ export default class ChallengeModel extends Model {
   }
 
   async _cloneAttachments(newChallenge) {
-    const files = this.hasMany('files').value() ?? [];
+    const files = (await this.files)?.slice() ?? [];
     files.map((attachment) => {
       const data = this._attachmentToJSON(attachment);
       this.store.createRecord('attachment', { ...data, challenge: newChallenge, cloneBeforeSave: true });

pix-editor/app/models/skill.js

@@ -175,8 +175,8 @@ export default class SkillModel extends Model {
   async pinRelationships() {
     const [tutoMore, tutoSolution] = await Promise.all([this.tutoMore, this.tutoSolution]);
     this._pinnedRelationships = {
-      tutoSolution: tutoSolution.slice(),
-      tutoMore: tutoMore.slice(),
+      tutoSolution: tutoSolution?.slice() ?? [],
+      tutoMore: tutoMore?.slice() ?? [],
     };
   }
 

pix-editor/tests/acceptance/create-challenge-test.js

@@ -46,27 +46,33 @@ module('Acceptance | Create-Challenge', function(hooks) {
     }
 
     this.owner.register('service:storage', StorageServiceStub);
-    const storageServiceStub = this.owner.lookup('service:storage');
-    sinon.stub(storageServiceStub, 'uploadFile').resolves({ url: 'data:,', filename: 'attachment-name' });
+    const storageService = this.owner.lookup('service:storage');
+    const illustrationFile = new File([], 'challenge-illustration.png', { type: 'image/png' });
+    const attachmentFile = new File([], 'challenge-attachment.csv', { type: 'text/csv' });
+    const uploadFileStub = sinon.stub(storageService, 'uploadFile');
+    uploadFileStub.withArgs({ file: sinon.match({ file: illustrationFile }) })
+      .resolves({ url: 'data_illustration:,', filename: 'illustration-name' });
+    uploadFileStub.withArgs({ file: sinon.match({ file: attachmentFile }), filename: 'challenge-attachment.csv', isAttachment: true })
+      .resolves({ url: 'data_attachment:,', filename: 'attachment-name' });
 
     // when
     await visit('/');
     await click(findAll('[data-test-area-item]')[0]);
     await click(findAll('[data-test-competence-item]')[0]);
     await click(find('.workbench'));
     await click(find('[data-test-create-new-challenge]'));
-    const file = new File([], 'challenge-illustration.png', { type: 'image/png' });
-    await selectFiles('[data-test-file-input-illustration] input', file);
+    await selectFiles('[data-test-file-input-illustration] input', illustrationFile);
+    await selectFiles('[data-test-file-input-attachment] input', attachmentFile);
     await click(find('[data-test-save-challenge-button]'));
 
-    const store = this.owner.lookup('service:store');
-
     // then
-    const attachments = await store.peekAll('attachment');
+    const store = this.owner.lookup('service:store');
+    const attachments = store.peekAll('attachment').slice();
     assert.dom('[data-test-main-message]').hasText('Prototype enregistré');
-    assert.ok(storageServiceStub.uploadFile.calledOnce);
+    assert.ok(uploadFileStub.calledTwice);
     assert.ok(attachments.every((record) => !record.isNew));
+    assert.strictEqual(attachments[0].url, 'data_illustration:,');
+    assert.strictEqual(attachments[1].url, 'data_attachment:,');
   });
-
 });
 

pix-editor/tests/acceptance/modify-challenge/modify-challenge-attachment-test.js

@@ -49,7 +49,7 @@ module('Acceptance | Modify-Challenge-Attachment', function(hooks) {
     await click(findAll('[data-test-skill-cell-link]')[0]);
     await click(find('[data-test-modify-challenge-button]'));
 
-    const file = new File([], 'challenge-illustration.png', { type: 'image/png' });
+    const file = new File([], 'challenge-attachment.png', { type: 'image/png' });
     await selectFiles('[data-test-file-input-attachment] input', file);
 
     await runTask(this, async () => { }, 400);
@@ -66,6 +66,51 @@ module('Acceptance | Modify-Challenge-Attachment', function(hooks) {
     assert.strictEqual(attachments.length, 1);
   });
 
+  test('replace attachment', async function(assert) {
+    // given
+    class StorageServiceStub extends Service {
+      uploadFile() { }
+    }
+
+    this.owner.register('service:storage', StorageServiceStub);
+    const storageService = this.owner.lookup('service:storage');
+    const attachmentA = new File([], 'challenge-attachmentA.png', { type: 'image/png' });
+    const attachmentB = new File([], 'challenge-attachmentB.png', { type: 'image/png' });
+    const uploadFileStub = sinon.stub(storageService, 'uploadFile');
+    uploadFileStub.withArgs({ file: sinon.match({ file: attachmentA }), filename: 'challenge-attachmentA.png', isAttachment: true }).resolves({ url: 'data-attachmentA:,', filename: 'attachment-nameA' });
+    uploadFileStub.withArgs({ file: sinon.match({ file: attachmentB }), filename: 'challenge-attachmentB.png', isAttachment: true }).resolves({ url: 'data-attachmentB:,', filename: 'attachment-nameB' });
+
+    // when
+    // adding attachmentA
+    await visit('/');
+    await click(findAll('[data-test-area-item]')[0]);
+    await click(findAll('[data-test-competence-item]')[0]);
+    await click(findAll('[data-test-skill-cell-link]')[0]);
+    await click(find('[data-test-modify-challenge-button]'));
+    await selectFiles('[data-test-file-input-attachment] input', attachmentA);
+    await runTask(this, async () => { }, 400);
+    await click(find('[data-test-save-challenge-button]'));
+    await click(find('[data-test-confirm-log-approve]'));
+
+    // replace attachmentA with attachmentB
+    await click(find('[data-test-modify-challenge-button]'));
+    await click(find('[data-test-delete-attachment-button]'));
+    await selectFiles('[data-test-file-input-attachment] input', attachmentB);
+    await runTask(this, async () => { }, 400);
+    await click(find('[data-test-save-challenge-button]'));
+    await click(find('[data-test-confirm-log-approve]'));
+
+    const store = this.owner.lookup('service:store');
+    const attachments = store.peekAll('attachment').slice();
+
+    // then
+    assert.dom('[data-test-main-message]').hasText('Épreuve mise à jour');
+    assert.ok(uploadFileStub.calledTwice);
+    assert.ok(attachments.every((record) => !record.isNew));
+    assert.strictEqual(attachments.length, 1);
+    assert.strictEqual(attachments[0].url, 'data-attachmentB:,');
+  });
+
   test('delete attachment', async function(assert) {
     // given
     this.server.create('challenge', {

pix-editor/tests/acceptance/modify-challenge/modify-challenge-illustration-test.js

@@ -65,6 +65,51 @@ module('Acceptance | Modify-Challenge-Illustration', function(hooks) {
     assert.ok(attachments.every((record) => !record.isNew));
   });
 
+  test('replace illustration', async function(assert) {
+    // given
+    class StorageServiceStub extends Service {
+      uploadFile() { }
+    }
+
+    this.owner.register('service:storage', StorageServiceStub);
+    const storageService = this.owner.lookup('service:storage');
+    const illustrationA = new File([], 'challenge-illustrationA.png', { type: 'image/png' });
+    const illustrationB = new File([], 'challenge-illustrationB.png', { type: 'image/png' });
+    const uploadFileStub = sinon.stub(storageService, 'uploadFile');
+    uploadFileStub.withArgs({ file: sinon.match({ file: illustrationA }) }).resolves({ url: 'data-illustrationA:,', filename: 'illustration-nameA' });
+    uploadFileStub.withArgs({ file: sinon.match({ file: illustrationB }) }).resolves({ url: 'data-illustrationB:,', filename: 'illustration-nameB' });
+
+    // when
+    // adding illustrationA
+    await visit('/');
+    await click(findAll('[data-test-area-item]')[0]);
+    await click(findAll('[data-test-competence-item]')[0]);
+    await click(findAll('[data-test-skill-cell-link]')[0]);
+    await click(find('[data-test-modify-challenge-button]'));
+    await selectFiles('[data-test-file-input-illustration] input', illustrationA);
+    await runTask(this, async () => { }, 400);
+    await click(find('[data-test-save-challenge-button]'));
+    await click(find('[data-test-confirm-log-approve]'));
+
+    // replace illustrationA with illustrationB
+    await click(find('[data-test-modify-challenge-button]'));
+    await click(find('[data-test-delete-illustration-button]'));
+    await selectFiles('[data-test-file-input-illustration] input', illustrationB);
+    await runTask(this, async () => { }, 400);
+    await click(find('[data-test-save-challenge-button]'));
+    await click(find('[data-test-confirm-log-approve]'));
+
+    const store = this.owner.lookup('service:store');
+    const attachments = store.peekAll('attachment').slice();
+
+    // then
+    assert.dom('[data-test-main-message]').hasText('Épreuve mise à jour');
+    assert.ok(uploadFileStub.calledTwice);
+    assert.ok(attachments.every((record) => !record.isNew));
+    assert.strictEqual(attachments.length, 1);
+    assert.strictEqual(attachments[0].url, 'data-illustrationB:,');
+  });
+
   test('delete illustration', async function(assert) {
     // given
     this.server.create('challenge', {