Add interface for model collection with tests #141

costascaio · 2020-07-22T20:03:47Z

No description provided.

rgaiacs

Great start. Some comments are related with style. Your code is valid but we prefer the other style.

I will add more comments later.

src/repositorios/collection.js

rgaiacs · 2020-07-22T20:38:09Z

src/repositorios/colllection.test.js

+    "Test CollectionModel",
+    () => {
+
+        /*


Remove this comment.

rgaiacs · 2020-07-22T20:40:28Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data)


This will fail because Sequelize is configured to return a query object instead of JSON. You need to use data.dataValues.

rgaiacs · 2020-07-22T20:42:58Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


You are expecting .toThrow(error?) instead of .toBeNull().

rgaiacs · 2020-07-22T20:43:10Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


You are expecting .toThrow(error?) instead of .toBeNull().

rgaiacs · 2020-07-22T20:43:53Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


The default should be public and the missing field should go to the default one.

rgaiacs · 2020-07-22T20:44:01Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


You are expecting .toThrow(error?) instead of .toBeNull().

rgaiacs · 2020-07-22T20:44:37Z

src/repositorios/colllection.test.js

+            }
+        );
+
+        /*


Start a new describe block.

rgaiacs · 2020-07-22T20:52:40Z

$ git remote -v           
caio	[email protected]:costascaio/searchable-image-database-nodejs.git (fetch)
caio	[email protected]:costascaio/searchable-image-database-nodejs.git (push)
origin	[email protected]:CRICDatabase/searchable-image-database-nodejs.git (fetch)
origin	[email protected]:CRICDatabase/searchable-image-database-nodejs.git (push)

@costascaio I rebased the origin/collection to origin/master without hit any conflict using

$ git checkout origin/collection
$ git rebase origin/master

Your pull request is now against the rebased version.

rgaiacs

You changed src/repositorios/injury.test.js but not src/repositorios/image.test.js.

rgaiacs · 2020-07-22T20:55:03Z

src/repositorios/injury.test.js

@@ -14,7 +14,8 @@ describe(
                    {
                        nome: "Short",
                        detalhes: "Long",
-                        grade: 100
+                        grade: 100,
+                        id_collection: 1


Test might fail if collection doesn't exist. Fix it using #136.

rgaiacs · 2020-07-22T20:55:11Z

src/repositorios/injury.test.js

@@ -25,7 +26,8 @@ describe(
                                        id: expect.any(Number),
                                        nome: "Short",
                                        detalhes: "Long",
-                                        grade: 100
+                                        grade: 100,
+                                        id_collection: 1


Test might fail if collection doesn't exist. Fix it using #136.

rgaiacs · 2020-07-22T20:55:17Z

src/repositorios/injury.test.js

@@ -47,7 +49,8 @@ describe(
                                        id: 1,
                                        nome: expect.any(String),
                                        detalhes: expect.any(String),
-                                        grade: expect.any(Number)
+                                        grade: expect.any(Number),
+                                        id_collection: expect.any(Number)


Test might fail if collection doesn't exist. Fix it using #136.

rgaiacs · 2020-07-22T20:55:24Z

src/repositorios/injury.test.js

@@ -87,7 +90,8 @@ describe(
                                            id: expect.any(Number),
                                            nome: expect.any(String),
                                            detalhes: expect.any(String),
-                                            grade: expect.any(Number)
+                                            grade: expect.any(Number),
+                                            id_collection: expect.any(Number)


Test might fail if collection doesn't exist. Fix it using #136.

rgaiacs · 2020-07-23T15:16:34Z

src/repositorios/imagem_repositorio.js

@@ -93,23 +94,25 @@ module.exports = {
        });
    },

-    async cadastrarCelulaSegmentada(id_imagem, id_descricao) {
+    async cadastrarCelulaSegmentada(id_imagem, id_descricao, id_collection) {


cadastrarCelulaSegmentada doesn't need id_collection. Check the model!

rgaiacs · 2020-07-23T15:17:02Z

src/repositorios/imagem_repositorio.js

        });
    },

-    async cadastrarCelulaClassificada(id_imagem, id_lesao) {
+    async cadastrarCelulaClassificada(id_imagem, id_lesao, id_collection) {


This function was removed during the simplification of the database schema.

rgaiacs · 2020-07-23T15:19:58Z

src/repositorios/colllection.test.js

@@ -0,0 +1,162 @@
+"use strict";


File name has a typo. Rename it from src/repositorios/colllection.test.js to src/repositorios/collection.test.js.

rgaiacs

Use sed -i 's/expecte/expect/' src/repositorios/colllection.test.js to fix the typos.

rgaiacs · 2020-07-23T15:21:15Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data)


Typo here. Replace expecte with expect.

rgaiacs · 2020-07-23T15:21:34Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


Typo here. Replace expecte with expect.

rgaiacs · 2020-07-23T15:21:45Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


Typo here. Replace expecte with expect.

rgaiacs · 2020-07-23T15:21:54Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


Typo here. Replace expecte with expect.

rgaiacs · 2020-07-23T15:22:02Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


Typo here. Replace expecte with expect.

rgaiacs · 2020-07-23T15:22:11Z

src/repositorios/colllection.test.js

+                })
+                .then(
+                    data => {
+                        expecte(data).toBeNull();


Typo here. Replace expecte with expect.

rgaiacs · 2020-08-17T19:39:59Z

@costascaio I rebased the collections branch in this repository. You must update your local copy:

$ git remote -v
origin	[email protected]:CRICDatabase/searchable-image-database-nodejs.git (fetch)
origin	[email protected]:CRICDatabase/searchable-image-database-nodejs.git (push)
$ git fetch origin
$ git checkout collection
$ git rebase origin/collections

And resolve the conflicts.

rgaiacs · 2020-08-20T18:10:38Z

src/repositorios/collection.js

+        return CollectionModel.findByPk(id_collection);
+    },
+
+    async update_collection(request){


Please add toggle_public_collection(id_collection).

rgaiacs · 2020-08-20T18:11:38Z

src/repositorios/collection.test.js

+                return CollectionDOA.create_collection({
+                    name: "Jest",
+                    slang: "jest",
+                    public: true,


The default for public should be false.

rgaiacs · 2020-08-20T18:12:25Z

src/repositorios/collection.test.js

+                )
+            }
+        );
+


create_collection without public is missing.

rgaiacs · 2020-08-20T18:14:06Z

src/repositorios/collection.test.js

+            }
+        );
+
+        test(


Move get_collection to their own suite (inside describe).

rgaiacs · 2020-08-20T18:15:14Z

src/repositorios/collection.test.js

+                            expect(data.dataValues)
+                                .toMatchObject(
+                                    {
+                                        id: expect.any(Number),


Replace

id: expect.any(Number),

with

id: 1,

rgaiacs · 2020-08-20T18:26:54Z

src/repositorios/collection.js

+        return CollectionModel.create(collection);
+    },
+
+    async delete_collection(id_collection){


We want to mark for deletion instead of delete. Need to add a new field delete.

rgaiacs · 2020-08-20T18:27:46Z

src/repositorios/collection.test.js

+        );
+    }
+)
+


You missed test delete_collection.

rgaiacs · 2020-08-20T18:29:49Z

src/repositorios/image.test.js

@@ -246,72 +263,3 @@ describe(

    }
 );
-
-
-describe(


This code should not be delete.

rgaiacs · 2020-09-07T13:49:14Z

@costascaio Any update?

rgaiacs · 2020-09-21T17:51:04Z

@costascaio you merge instead of rebase. Please fix it.

rgaiacs · 2020-09-24T14:26:48Z

@costascaio please follow the steps

$ git remote -v
caio	[email protected]:costascaio/searchable-image-database-nodejs.git (fetch)
caio	[email protected]:costascaio/searchable-image-database-nodejs.git (push)
origin	[email protected]:CRICDatabase/searchable-image-database-nodejs.git (fetch)
origin	[email protected]:CRICDatabase/searchable-image-database-nodejs.git (push)
$ git checkout origin/master
$ git branch -f master
$ git branch -f collection
$ git cherry-pick e900588
$ git cherry-pick fa0e227
$ git cherry-pick 9c5f7fe
```

You need to fix the conflict in `src/repositorios/image.test.js`.

```
$ git add src/repositorios/image.test.js
$ git commit
$ git cherry-pick 85583d0
```

You need to resolve `src/models/CollectionModel.js` and `src/repositorios/image.test.js`.

```
$ git add
$ git commit
$ git cherry-pick 30fa851
```

You need to resolve `src/fixtures/collection.json`.

```
$ git add
$ git commit
$ git push -f caio collection
```

costascaio self-assigned this Jul 22, 2020

rgaiacs changed the base branch from master to collections July 22, 2020 20:31

rgaiacs added the enhancement New feature or request label Jul 22, 2020

rgaiacs added this to the v3.0.0 milestone Jul 22, 2020

rgaiacs linked an issue Jul 22, 2020 that may be closed by this pull request

Add interface for model collection #84

Open

4 tasks

rgaiacs requested changes Jul 22, 2020

View reviewed changes

rgaiacs mentioned this pull request Jul 22, 2020

Add fixture for collections #136

Open

2 tasks

rgaiacs requested changes Jul 22, 2020

View reviewed changes

rgaiacs requested changes Jul 23, 2020

View reviewed changes

rgaiacs reviewed Jul 23, 2020

View reviewed changes

costascaio and others added 9 commits July 23, 2020 16:59

add collection model

dbfd02d

assing descricao, lesao and image to cervix collection

3774dfa

update migrations descricao, lesao and image

6ff110d

fix comments from pull request

812abc2

rename migrations files

3e51b64

Rename migration file

9d4d7fa

Rename migration files

adeb2b8

Make collection private by default

c1e24c7

Lint files

a0027d1

rgaiacs force-pushed the collections branch from c6357ca to fc06be0 Compare July 29, 2020 18:39

costascaio force-pushed the collection branch from 0f86479 to e1af611 Compare August 14, 2020 22:02

rgaiacs force-pushed the collections branch from fc06be0 to 11c2e59 Compare August 17, 2020 19:37

costascaio force-pushed the collection branch from e1af611 to ce16e4f Compare August 18, 2020 16:09

rgaiacs requested changes Aug 20, 2020

View reviewed changes

costascaio force-pushed the collection branch from ce16e4f to 28a703d Compare September 17, 2020 19:14

costascaio requested a review from rgaiacs September 21, 2020 12:52

costascaio force-pushed the collection branch 2 times, most recently from 3cb42f9 to a0027d1 Compare September 24, 2020 18:56

costascaio and others added 4 commits September 24, 2020 16:40

improve tests from collection.test.js

2caf874

fix tests from get_collection and delete_collection

ebfed07

fix comments from 141

ad18d4d

Merge branch 'collections' into collection

e762cae

+                          }
+                      );
+                      /*

+                          }
+                      );
+                      test(

Add interface for model collection with tests #141

Are you sure you want to change the base?

Add interface for model collection with tests #141

Conversation

costascaio commented Jul 22, 2020

rgaiacs 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

rgaiacs commented Jul 22, 2020

rgaiacs 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

rgaiacs 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

rgaiacs commented Aug 17, 2020

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

rgaiacs commented Sep 7, 2020

rgaiacs commented Sep 21, 2020

rgaiacs commented Sep 24, 2020