implement PUT /api/0.6/changeset/create #7

galta95 · 2020-11-26T08:14:24Z

Closes #2

chore(package): merge ci changes from master

syncush

your tests folder is inside src, it should be outside.

syncush · 2020-11-26T08:37:44Z

src/tests/unit/helper/nock-apiv6.ts

+export const createChangesetNock = (): void => {
+    nock(`${host}:${port}`)
+    .put(`${createChangesetEndPoint}`)
+    .reply(200, '12');
+};


the function name does not indicate whether if the nock is for success or failure.

removed this and transfered to the test itself

syncush · 2020-11-26T08:38:13Z

src/tests/unit/helper/nock-apiv6.ts

+
+export const createChangesetNock = (): void => {
+    nock(`${host}:${port}`)
+    .put(`${createChangesetEndPoint}`)


no need for a string template if you are using a string variable already

syncush · 2020-11-26T08:39:30Z

.eslintrc.js

        "@typescript-eslint"
    ],
    "rules": {
+        semi: [2, "always"]
    }
 };


you didn't use our eslint-config https://github.com/MapColonies/eslint-config

syncush · 2020-11-26T08:43:15Z

index.ts

+import Apiv6 from './src/api/v6';
+
+export default Apiv6;


when you bundle the code, you should bundle the code under src folder -> this file should be under /src

syncush · 2020-11-26T08:44:58Z

package.json

    "axios": "^0.21.0",
-    "typescript": "^4.1.2"
+    "typescript": "^4.1.2",


typescript should be a devDep

syncush · 2020-11-26T08:49:49Z

src/api/v6/index.ts

+    }
+    setUserName(username: string): boolean {
+         this.username = username;
+         return true;


why do we need to return true?

nevermind ...fixed

syncush · 2020-11-26T08:50:27Z

src/api/v6/index.ts

+    setUserName(username: string): boolean {
+         this.username = username;
+         return true;
+    }
+    setPassword(password: string): boolean {
+        this.password = password;
+        return true;
+    }


I would drop the two function in favor of setCreds(username, password)

syncush · 2020-11-26T08:52:41Z

src/api/v6/index.ts

+        catch (e) {
+            const { response: { status: code, data: message }  } = e;
+            return response(code, message);
+        }


We have talked about going over the responses of the API by their spec and create meaningful Exceptions like 400 bad request or 405 method not allowed

syncush · 2020-11-26T09:03:15Z

src/tests/unit/unit.test.ts

@@ -0,0 +1,43 @@
+import { expect } from 'chai';
+import { describe, it } from 'mocha';


I don't think you need to import describe and it once you run mocha you get them as a global function, if it is because you had a listing problem you need to specify in eslintrc that you use mocha

right, done

syncush · 2020-11-26T09:04:02Z

src/tests/unit/unit.test.ts

+                before(async function () {
+                    createChangesetNockNotAuth();
+                });


you use an async function but does not use await inside, it is redundant

syncush · 2020-11-26T09:15:48Z

Please add prettier to the project

syncush · 2020-11-29T07:17:43Z

package.json

@@ -4,7 +4,7 @@
  "description": "easy communication with osm api",
  "main": "index.js",
  "scripts": {
-    "test": "mocha -r ts-node/register 'src/tests/unit/unit.test.ts'",
+    "test": "mocha -r ts-node/register 'tests/**/*.ts'",


the test file pattern should be tests/**/*.test.ts

syncush · 2020-11-29T07:41:13Z

tests/unit/unit.test.ts

+import { expect } from 'chai';
+import nock = require('nock');
+
+import Apiv6 from '../../src/index';
+import { testConf } from './config/tests-config';
+import { createChangesetEndPoint } from '../../src/lib/endpoints';
+
+const { url, username, password } = testConf;
+
+describe('apiv6', async function () {
+    const apiv6 = new Apiv6(url, username, password);
+    describe('happy flow', async function () {
+        describe('/changeset/create', async function () {
+            describe('with register user', async function () {
+                it('should return 200 and changset number', async function () {
+                    nock(url).put(createChangesetEndPoint).reply(200, '12');
+                    const res = await apiv6.createChangeset("test-generator", "test-user");
+
+                    expect(res).to.be.a('object')
+                                        .with.property('code')
+                                        .and.to.be.equal(200);
+                    expect(res).to.have.property('message')
+                            .and.to.be.equal(12);
+                });
+            });
+            describe('with unregisterd user', async function () {
+                it('should return error', async function () {
+                    nock(url).put(createChangesetEndPoint).reply(401, "Couldn't authenticate you");
+                    apiv6.setCreds('not-registerd', '123456');
+
+                    try {
+                        await apiv6.createChangeset("test-generator", "test-user");
+                    } catch (e) {
+                        expect(e).to.be.a('Error')
+                                .with.property('message')
+                                .and.to.be.equal('Couldn\'t authenticate you');
+                        expect(e).to.have.property('status')
+                                 .and.to.be.equal(401);
+                    }
+                });
+            });
+        });
+    });
+});


the file name should be apiv6.test.ts

…atch

syncush · 2020-11-29T11:38:19Z

package.json

@@ -29,9 +32,7 @@
  "homepage": "https://github.com/MapColonies/node-osm-api#readme",
  "dependencies": {
    "@types/node": "^14.14.9",


types node should be devDep

syncush

should run lint on test files

* ci(tests): added unit tets * fix(github- workflows): split to two different .yml files for each job category

* feat(/api/0.6/changeset/create): create changeset (#7) * feat(apiv6): implemented closeChangeset (#12) * feat(apiv6): implemented upload Changeset (#14)

galta95 added 6 commits November 23, 2020 13:55

feat(/api/0.6/changeset/create): new rout

ea0d42d

test(/api/0.6/changeset/create): unit test

a427f41

feat(eslint): added semicolon role

56733b0

style(lint): missing semicolons and change file name

39b71a1

build(npm): added nock

5b570b2

Merge branch 'master' into create-changeset-rout

a2bf377

chore(package): merge ci changes from master

galta95 requested review from melancholiai and syncush November 26, 2020 08:14

syncush mentioned this pull request Nov 26, 2020

implement changeset API of API v0.6 #10

Closed

style(lint): added semicolon

05098c9

syncush requested changes Nov 26, 2020

View reviewed changes

refactor(/api/0.6/changeset/create): refactor requested changes

014d8c7

syncush requested changes Nov 29, 2020

View reviewed changes

galta95 added 2 commits November 29, 2020 09:51

fix(/api/0.6/changeset/create): move out the res parsing from try & c…

b6cdffd

…atch

refactor(all): use generics

8718281

syncush requested changes Nov 29, 2020

View reviewed changes

galta95 force-pushed the create-changeset-rout branch from 717d2a7 to 45e5e8f Compare November 29, 2020 13:12

refactor(/api/0.6/changeset/create): error handler

a78eca8

galta95 force-pushed the create-changeset-rout branch from 45e5e8f to a78eca8 Compare November 29, 2020 13:33

syncush requested changes Nov 29, 2020

View reviewed changes

ci(tests): added unit tests and lint workflows (#11)

57f1734

* ci(tests): added unit tets * fix(github- workflows): split to two different .yml files for each job category

galta95 force-pushed the create-changeset-rout branch 8 times, most recently from 461f2af to e6ca108 Compare November 29, 2020 14:35

galta95 force-pushed the create-changeset-rout branch 4 times, most recently from 1b1b7c9 to 6255647 Compare November 29, 2020 14:50

refactor(.github/workflows): added linting to tests directories

350eaca

galta95 force-pushed the create-changeset-rout branch from 6255647 to 350eaca Compare November 29, 2020 14:55

syncush approved these changes Nov 29, 2020

View reviewed changes

syncush merged commit aef079b into apiv6 Nov 29, 2020

galta95 deleted the create-changeset-rout branch December 3, 2020 14:48

galta95 mentioned this pull request Dec 3, 2020

implement changeset API of API v0.6 #15

Merged

syncush pushed a commit that referenced this pull request Dec 3, 2020

feat(apiv6): implement changeset API of APIv0.6 (#15)

cfed1cb

* feat(/api/0.6/changeset/create): create changeset (#7) * feat(apiv6): implemented closeChangeset (#12) * feat(apiv6): implemented upload Changeset (#14)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement PUT /api/0.6/changeset/create #7

implement PUT /api/0.6/changeset/create #7

galta95 commented Nov 26, 2020 •

edited

Loading

syncush left a comment

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush Nov 26, 2020

galta95 Nov 26, 2020

syncush commented Nov 26, 2020 •

edited

Loading

syncush Nov 29, 2020

syncush Nov 29, 2020

syncush Nov 29, 2020

syncush left a comment

		@@ -0,0 +1,43 @@
		import { expect } from 'chai';
		import { describe, it } from 'mocha';

implement PUT /api/0.6/changeset/create #7

implement PUT /api/0.6/changeset/create #7

Conversation

galta95 commented Nov 26, 2020 • edited Loading

syncush 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

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

syncush commented Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syncush left a comment

Choose a reason for hiding this comment

galta95 commented Nov 26, 2020 •

edited

Loading

syncush commented Nov 26, 2020 •

edited

Loading