Add a ressource to an item (see #142). #598

dylanbonelli · 2022-05-12T15:11:01Z

Co-authored-by: corentinprp51 [email protected]
Co-authored-by: Caramella-ICE [email protected]

Content

Please describe your contribution here...

Checklist

Please check that your pull request is correct:

Each commit:
- corresponds to a contribution that should be notified to users,
- does not generate new errors or warnings at compile or test time,
- must be attributed to its real authors (with correct GitHub IDs and correct syntax for multiple authors).
The title of a commit should:
- begin with a contribution type
  - FEATURE for a behaviour allowing a user to do something new,
  - FIX for a behaviour which has been changed in order to meet user’s expectations,
  - SCENARIO for examples showing a given behaviour,
  - TEST when it concerns an acceptance test of a given behaviour,
  - PROCESS for a change in the way the software is built, tested, deployed,
  - DOC when it concerns only internal documentation (however it is better to combine it with the contribution that required this documentation change),
- be followed by a colon (:) with one space after and no space before,
- be followed by a title (written in English) as short, as user-centered and as explicit as possible
  - If it is a feature, the title must be the user action (beginning with a verb, and please not manage),
  - If it is a fix, the title must describe the intended behavior (with should).
- ends with a reference to the corresponding ticket with the following syntax:
  - (closes #xx) if xx is a feature ticket (and the commit is a complete implementation),
  - (fixes #xx) if xx is a fix ticket (and the commit is a complete fix),
  - (see #xx) otherwise,
Each committed line is:
- useful (it would not work if removed)
  - if it is a comment line, its information could not be conveyed by better variables and function naming, better code structuring, or better commit message,
- related to this very contribution (feature, fix...),
- in English (with the exception of Gherkin scenarios in French and resulting steps),
- without any typo in variable, class or function names,
- correctly indented (spaces rather than tabs, same number of characters as in the rest of the file).

benel

Thank you for this important contribution.

Here are the few problems that should be fixed before integration.

Please also verify the coauthors mentioned in commits (they are not the same as in the pull request).

benel · 2022-06-01T17:44:45Z

src/components/itemPage/InputFile.jsx

+    const myHeaders = new Headers();
+    myHeaders.append('Content-Type', file.type);
+    myHeaders.append('credentials', 'include');
+    myHeaders.append('Authorization', 'Basic ' + Buffer.from('alice:whiterabbit').toString('base64'));


That shouldn't be hard-coded.

benel · 2022-06-01T17:51:38Z

features/step_definitions/event.rb

+end
+
+Quand("l'utilisateur consulte la ressource {string}") do |string|
+#   click_on string


Commenting out a line of code is considered as a bad practice.

benel · 2022-06-01T17:52:35Z

src/components/itemPage/InputFile.jsx

@@ -0,0 +1,59 @@
+import React, {useEffect, useState} from 'react';
+// import Hypertopic from 'hypertopic';


Commenting out a line of code is considered as a bad practice.

benel · 2022-06-01T17:54:28Z

src/components/itemPage/InputFile.jsx

+  const [rev, setRev] = useState('');
+  const [uploadStatus, setUpload] = useState({ isUpload: false, message: '' });
+
+  // Post one file


If you think the name of the function is not clear enough, please change it instead of adding such a comment.

benel · 2022-06-01T17:55:20Z

src/components/itemPage/InputFile.jsx

+
+  // Post one file
+  const postFile = async (file, revision) => {
+    // Gestion des headers pour la requête


If the comment is really needed for developpers, it should be translated in the same language of the code.

benel · 2022-06-01T17:58:33Z

features/add_ressource_to_item.feature

+Soit "AXN 009" l'item affiché
+Et l'utilisateur est connecté
+Quand l'utilisateur dépose "favicon.ico" comme ressource
+Alors la ressource "favicon.ico" est ajoutée


You should test a state rather than an action. Please rephrase "est ajoutée".

benel · 2022-06-01T18:00:11Z

features/add_ressource_to_item.feature

+Scénario: Consulter une ressource en tant que fichier 
+
+Soit "AXN 009" l'item affiché
+Et la ressource "favicon.ico" existe comme ressource


This is considered as a bad practice to make a test depend on the success of a previous test.
Either find a way to include the attachment in the test data. Or just remove this scenario (and the corresponding steps).

benel · 2022-06-01T18:01:16Z

features/add_ressource_to_item.feature

+Alors la ressource "favicon.ico" est ajoutée
+
+
+Scénario: Consulter une ressource en tant que fichier 


This is not really a different case of the feature. Please move it to the consultation of an item or just remove this scenario.

benel · 2022-06-01T18:02:28Z

features/add_ressource_to_item.feature

+Fonctionnalité: Ajouter une ressource en tant que fichier 
+
+
+Scénario: Ajouter une ressource en tant que fichier 


The name of a scenario should correspond to a special case of the feature.
Duplicating the feature name just means that you should let the scenario title empty.

benel · 2022-06-02T09:03:37Z

src/components/itemPage/InputFile.jsx

+  return (
+    <div>
+      <div >
+        <button className="inputFiles_Button" onClick={()=>document.getElementById('inputFiles').click() } >Ajouter une ressource </button>


Please use lingui.js to internationalize the label.

benel · 2022-06-02T09:05:58Z

src/components/itemPage/Item.jsx

@@ -89,6 +90,11 @@ class Item extends Component {
                    <div className="d-none d-sm-block">{this._getAttributeCreationForm()}</div>
                  </div>
                  {viewpoints}
+                  <div className="Ressources">
+                    <h3 className="h4"><Trans>Ressources</Trans></h3>


Please use npm extract to create the entry in the localization dictionaries and then translate it.

benel · 2022-06-02T09:12:22Z

src/components/itemPage/InputFile.jsx

+    event.preventDefault();
+    const files = Array.from(event.target.files);
+    let newRev = rev;
+    for (const file of files) {


const ? N'est-ce pas un peu contradictoire ?

benel · 2022-06-02T09:14:42Z

src/components/itemPage/InputFile.jsx

+    for (const file of files) {
+      let myHeaders = new Headers();
+      myHeaders.append('Content-Type', file.type);
+      myHeaders.append('credentials', 'include');


Que faites-vous ensuite de ces entêtes ?

benel · 2022-06-02T09:15:58Z

src/components/itemPage/InputFile.jsx

+      newRev = result.rev;
+    }
+    if (!uploadStatus.message) {
+      setUpload({ isUpload: true, message: 'Tous les fichiers ont bien été ajoutés' });


Si vraiment vous voulez afficher un message, il faut le "localiser"...
Par contre, on peut se demander si c'est nécessaire, puisque le résultat doit normalement déjà se voir.

benel · 2022-06-02T09:19:21Z

src/components/itemPage/InputFile.jsx

+    return await fetch(await service + itemId + '/' + file.name + '?rev=' + revision,
+      { method: 'PUT', body: file, headers: myHeaders})
+      .then(res => res.json())
+      .catch(() => setUpload({ isUpload: false, message: 'Une erreur est survenue' }));


"Localiser" le message.

Co-authored-by: corentinprp51 <[email protected]> Co-authored-by: Caramella-ICE <[email protected]>

Co-authored-by: corentinprp51 <[email protected]>

dylanbonelli force-pushed the feature-142 branch 5 times, most recently from 36e4226 to 3f1d108 Compare May 19, 2022 15:11

dylanbonelli marked this pull request as ready for review May 19, 2022 15:28

corentinprp51 requested a review from benel May 22, 2022 18:06

benel requested changes Jun 1, 2022

View reviewed changes

benel reviewed Jun 2, 2022

View reviewed changes

dylanbonelli and others added 3 commits June 2, 2022 14:35

SCENARIO: add a ressource to an item (see Hypertopic#142).

9119e74

Co-authored-by: corentinprp51 <[email protected]> Co-authored-by: Caramella-ICE <[email protected]>

TEST: add test for a ressource to an item.

725b06b

Co-authored-by: corentinprp51 <[email protected]>

FEATURE: add a resource to an item.

881f595

Co-authored-by: corentinprp51 <[email protected]>

dylanbonelli force-pushed the feature-142 branch from 3f1d108 to 881f595 Compare June 2, 2022 14:46

benel merged commit f9f8b18 into Hypertopic:v7 Jun 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a ressource to an item (see #142). #598

Add a ressource to an item (see #142). #598

dylanbonelli commented May 12, 2022 •

edited

Loading

benel left a comment

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 1, 2022

benel Jun 2, 2022

benel Jun 2, 2022

benel Jun 2, 2022

benel Jun 2, 2022

benel Jun 2, 2022

benel Jun 2, 2022

		@@ -0,0 +1,59 @@
		import React, {useEffect, useState} from 'react';
		// import Hypertopic from 'hypertopic';

		Alors la ressource "favicon.ico" est ajoutée


		Scénario: Consulter une ressource en tant que fichier

		Fonctionnalité: Ajouter une ressource en tant que fichier


		Scénario: Ajouter une ressource en tant que fichier

Add a ressource to an item (see #142). #598

Add a ressource to an item (see #142). #598

Conversation

dylanbonelli commented May 12, 2022 • edited Loading

Content

Checklist

benel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylanbonelli commented May 12, 2022 •

edited

Loading