[#175441170] Add Organization and Service logo's upload #110

infantesimone · 2020-11-09T12:25:52Z

This PR introduces two new endpoints, according to the following definition:

/services/{service_id}/logo:
    parameters:
      - name: service_id
        in: path
        type: string
        required: true
        description: The ID of an existing Service.
    put:
      summary: Upload service logo.
      description: |
        Upsert a logo for an existing service.
      parameters:
        - name: body
          in: body
          required: true
          schema:
            $ref: "#/definitions/Logo"
          description: A base64 string representation of the service logo PNG image.
            It can be the service own logo or the organization logo.
      responses:
        "200":
          description: Logo uploaded.
        "400":
          description: Invalid payload.
        "401":
          description: Unauthorized.
        "403":
          description: Forbidden.
        "404":
          description: No service found for the provided ID.
        "500":
          description: The service logo cannot be uploaded.
/organizations/{organization_fiscal_code}/logo:
     - name: organization_fiscal_code
        in: path
        type: string
        required: true
        description: organization fiscal code.
    put:
      summary: Upload organization logo.
      description: |
        Upsert a logo for an Organization.
      parameters:
        - name: body
          in: body
          required: true
          schema:
            $ref: "#/definitions/Logo"
          description: A base64 string representation of the organization logo PNG image.
      responses:
        "202":
          description: Logo uploaded.
        "400":
          description: Invalid payload
        "401":
          description: Unauthorized.
        "403":
          description: Forbidden.
        "429":
          description: Too many requests.
        "500":
          description: The organization logo cannot be uploaded.
  Logo:
    type: object
    properties:
      logo:
        type: string
        format: byte
        minLength: 1
    required:
      - logo

pagopa-github-bot · 2020-11-09T12:27:08Z

Affected stories

🌟 #175441170: Come ADM, vorrei poter caricare i loghi di servizi e organizzazioni, in modo autonomo, dal developer portal

New dependencies added: @types/jest, jest and ts-jest.

@types/jest

Author: Unknown

Description: TypeScript definitions for Jest

Homepage: http://npmjs.com/package/@types/jest


Created	over 4 years ago
Last Updated	3 days ago
License	MIT
Maintainers	1
Releases	143
Direct Dependencies	`jest-diff` and `pretty-format`

README

Installation

npm install --save @types/jest

Summary

This package contains type definitions for Jest (https://jestjs.io/).

Details

Files were exported from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/jest.

Additional Details

Last updated: Tue, 20 Oct 2020 07:15:43 GMT
Dependencies: @types/jest-diff, @types/pretty-format
Global values: afterAll, afterEach, beforeAll, beforeEach, describe, expect, fail, fdescribe, fit, it, jasmine, jest, pending, spyOn, test, xdescribe, xit, xtest

Credits

These definitions were written by Asana (https://asana.com)
// Ivo Stratev, jwbay, Alexey Svetliakov, Alex Jover Morales, Allan Lukwago, Ika, Waseem Dahman, Jamie Mason, Douglas Duteil, Ahn, Josh Goldberg, Jeff Lau, Andrew Makarov, Martin Hochel, Sebastian Sebald, Andy, Antoine Brault, Gregor Stamać, ExE Boss, Alex Bolenok, Mario Beltrán Alarcón, Tony Hallett, Jason Yu, Devansh Jethmalani, Pawel Fajfer, and Regev Brody.

jest

Author: Unknown

Description: Delightful JavaScript Testing.

Homepage: https://jestjs.io/


Created	over 8 years ago
Last Updated	15 days ago
License	MIT
Maintainers	8
Releases	261
Direct Dependencies	`@jest/core`, `import-local` and `jest-cli`
Keywords	ava, babel, coverage, easy, expect, facebook, immersive, instant, jasmine, jest, jsdom, mocha, mocking, painless, qunit, runner, sandboxed, snapshot, tap, tape, test, testing, typescript and watch

README

Jest

🃏 Delightful JavaScript Testing

👩🏻‍💻 Developer Ready: Complete and ready to set-up JavaScript testing solution. Works out of the box for any React project.
🏃🏽 Instant Feedback: Failed tests run first. Fast interactive mode can switch between running all tests or only test files related to changed files.
📸 Snapshot Testing: Jest can capture snapshots of React trees or other serializable values to simplify UI testing.

ts-jest

Author: Kulshekhar Kabra

Description: A preprocessor with source maps support to help use TypeScript with Jest

Homepage: https://kulshekhar.github.io/ts-jest


Created	about 4 years ago
Last Updated	8 days ago
License	MIT
Maintainers	3
Releases	133
Direct Dependencies	`@types/jest`, `bs-logger`, `buffer-from`, `fast-json-stable-stringify`, `jest-util`, `json5`, `lodash.memoize`, `make-error`, `mkdirp`, `semver` and `yargs-parser`
Keywords	jest, typescript, sourcemap, react and testing

Generated by 🚫 dangerJS

AleDore · 2020-11-09T14:26:18Z

src/app.ts

+    withRequestMiddlewares(
+      getApiClientMiddleware(),
+      getUserFromRequestMiddleware(),
+      RequiredParamMiddleware("serviceId", NonEmptyString),


Can we use ServiceId as type instead of NonEmptyString ?

AleDore · 2020-11-09T14:27:39Z

src/app.ts

+    withRequestMiddlewares(
+      getApiClientMiddleware(),
+      getUserFromRequestMiddleware(),
+      RequiredParamMiddleware("organizationFiscalCode", NonEmptyString),


Can we use ÒrganizationFiscalCode as type ?

AleDore · 2020-11-09T14:34:53Z

src/controllers/services.ts

+    .chain(user =>
+      checkAdminTask(user).chain(() =>
+        uploadOrganizationLogoTask(organizationFiscalCode, serviceLogo).map(
+          result => result
+        )
+      )


Can we flat this chain's pipeline ? i.e:

Suggested change

.chain(user =>

checkAdminTask(user).chain(() =>

uploadOrganizationLogoTask(organizationFiscalCode, serviceLogo).map(

result => result

)

)

.chain(user =>

checkAdminTask(user)

).chain(() =>

uploadOrganizationLogoTask(organizationFiscalCode, serviceLogo)

)

AleDore · 2020-11-09T14:35:48Z

src/controllers/services.ts

+  return getApimUserTask(apiClient, authenticatedUser)
+    .chain(user =>
+      checkAdminTask(user).chain(() =>
+        uploadOrganizationLogoTask(organizationFiscalCode, serviceLogo).map(


Why map without change right's value?

AleDore · 2020-11-09T14:43:14Z

src/middlewares/upload_logo.ts

+      response.isSome()
+        ? taskEither.of(response.value)
+        : fromLeft(
+            ResponseErrorNotFound(
+              "API user not found",
+              "Cannot find a user in the API management with the provided email address"
+            )
+          )


Since response is an option we should name it as maybeResponse. Aside can we use fold?

Thanks,
fold is better.

AleDore · 2020-11-09T14:46:26Z

src/middlewares/upload_logo.ts

+  ).foldTaskEither(
+    error => fromLeft(error),
+    errorOrResponse =>
+      errorOrResponse && errorOrResponse.status === 201


Could errorOrResponse be undefined ?

Yes, can be undefined according to uploadServiceLogo defined in api_client.ts.

Ok so if it can ben undefined, maybe we can wrap it into an option with fromNullable :)

That's something that does not sound good to me. I think that the response body could be undefined, so the entire either errorOrResponse could not imho.

The methods defined in api-client.ts return :

Promise<IResponseType<404, Error, never> | ... 4 more ... | undefined>
so the entire errorOrResponse can be undefined.
The code to avoid undefined could impact all methods of api-client.ts (maybe we could do it in a dedicated PR)

To use this definition I have to change apiResponseDecodeer, or maybe I create one to use only for new methods ?

The first one :). Let's keep in mind that probably we should use io-utils to generate clients in this project cc @balanza @gunzip

In this project there is already a client that is not generated with io-utils. I added the service to this but the problem is that in case of 400, the body is decoded as Error while in the model generated with io-utils is decoded as ProblemJson.

However undefined depends on how the client is defined in this project, there is a utility method that does a similar thing:

io-developer-portal-backend/src/api_client.ts

Line 197 in 8b5ef26

export function toEither<T>(

Ok, for the moment let's use the utility to avoid directly check undefined response. Btw, when it's possible, we have to do another PR with io-utils upgrade 👍

AleDore · 2020-11-09T14:46:58Z

src/middlewares/upload_logo.ts

+  ).foldTaskEither(
+    err => fromLeft(err),
+    errorOrResponse =>
+      errorOrResponse && errorOrResponse.status === 201


Yes, (as for uploadServiceLogo) can be undefined according to uploadOrganizationLogo defined in api_client.ts.

AleDore · 2020-11-11T13:36:24Z

ping :)

balanza · 2020-11-19T14:28:29Z

ping :)

👆

infantesimone · 2020-11-19T15:02:30Z

if it's ok as I used fromnullable, the pr is done

gunzip · 2020-11-19T15:07:46Z

needs rebasing (see conflicts)

AleDore

LGTM, fromNullable it's ok for now.

infantesimone added 3 commits November 7, 2020 17:23

Add upload logo for services and organizations

3cb3b17

restore file .env.demo

0b42062

refactoring and add unit and integrations test

fd36768

infantesimone requested review from AleDore, balanza, gunzip and francescopersico November 9, 2020 13:35

AleDore reviewed Nov 9, 2020

View reviewed changes

infantesimone added 2 commits November 9, 2020 19:26

Fix according PR feedback

502885f

Use fromNullable for undefined check

0989b79

balanza and others added 5 commits November 19, 2020 16:22

add changelog generation

fbf2554

0.7.0

be43b1a

refactoring and add unit and integrations test

deb8416

fix yanr lock

bf0e290

Merge branch 'master' into 175441170-upload-logo

4d17aad

AleDore approved these changes Nov 19, 2020

View reviewed changes

balanza approved these changes Nov 19, 2020

View reviewed changes

gunzip approved these changes Nov 20, 2020

View reviewed changes

gunzip merged commit 4e2dc01 into master Nov 20, 2020

gunzip deleted the 175441170-upload-logo branch November 20, 2020 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#175441170] Add Organization and Service logo's upload #110

[#175441170] Add Organization and Service logo's upload #110

infantesimone commented Nov 9, 2020 •

edited

Loading

pagopa-github-bot commented Nov 9, 2020 •

edited

Loading

Installation

Summary

Details

Additional Details

Credits

Jest

AleDore Nov 9, 2020

infantesimone Nov 9, 2020

AleDore Nov 9, 2020

AleDore Nov 9, 2020

AleDore Nov 9, 2020

AleDore Nov 9, 2020

infantesimone Nov 9, 2020

AleDore Nov 9, 2020

infantesimone Nov 9, 2020

AleDore Nov 10, 2020

AleDore Nov 10, 2020

infantesimone Nov 10, 2020

infantesimone Nov 11, 2020

AleDore Nov 11, 2020

infantesimone Nov 11, 2020 •

edited

Loading

infantesimone Nov 11, 2020

AleDore Nov 12, 2020

AleDore Nov 9, 2020

infantesimone Nov 9, 2020

AleDore Nov 10, 2020

AleDore commented Nov 11, 2020

balanza commented Nov 19, 2020

infantesimone commented Nov 19, 2020

gunzip commented Nov 19, 2020

AleDore left a comment

[#175441170] Add Organization and Service logo's upload #110

[#175441170] Add Organization and Service logo's upload #110

Conversation

infantesimone commented Nov 9, 2020 • edited Loading

pagopa-github-bot commented Nov 9, 2020 • edited Loading

Affected stories

Installation

Summary

Details

Additional Details

Credits

Jest

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

infantesimone Nov 11, 2020 • edited Loading

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

AleDore commented Nov 11, 2020

balanza commented Nov 19, 2020

infantesimone commented Nov 19, 2020

gunzip commented Nov 19, 2020

AleDore left a comment

Choose a reason for hiding this comment

infantesimone commented Nov 9, 2020 •

edited

Loading

pagopa-github-bot commented Nov 9, 2020 •

edited

Loading

infantesimone Nov 11, 2020 •

edited

Loading