[#170333527] Service logo upload function #10

alexgpeppe · 2020-02-04T14:28:28Z

This PR aims to add a new function to upload the logo of an existing service.

…mons

…n value

…e() return value

digitalcitizenship · 2020-02-10T11:23:53Z

Affected stories

🌟 #170333527: Come frontend voglio poter caricare un nuovo logo dell'ente su uno storage permanente

New dependencies added: @types/lolex and lolex.

@types/lolex

Author: Unknown

Description: TypeScript definitions for lolex

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


Created	over 3 years ago
Last Updated	7 days ago
License	MIT
Maintainers	1
Releases	22
Direct Dependencies

README

Installation

npm install --save @types/lolex

Summary

This package contains type definitions for lolex (https://github.com/sinonjs/lolex).

Details

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

Additional Details

Last updated: Fri, 15 Nov 2019 20:14:14 GMT
Dependencies: none
Global values: none

Credits

These definitions were written by Wim Looman (https://github.com/Nemo157), Josh Goldberg (https://github.com/joshuakgoldberg), Rogier Schouten (https://github.com/rogierschouten), and Yishai Zehavi (https://github.com/zyishai).

lolex

Author: Christian Johansen

Description: Fake JavaScript timers

Homepage: http://github.com/sinonjs/lolex


Created	about 5 years ago
Last Updated	7 days ago
License	BSD-3-Clause
Maintainers	4
Releases	44
Direct Dependencies	`@sinonjs/commons`

This README is too long to show.

Generated by 🚫 dangerJS

codecov-io · 2020-02-10T11:24:26Z

Codecov Report

Merging #10 into master will decrease coverage by 1.06%.
The diff coverage is 81.25%.

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
- Coverage    86.5%   85.44%   -1.07%     
==========================================
  Files           7        8       +1     
  Lines         126      158      +32     
  Branches        8       10       +2     
==========================================
+ Hits          109      135      +26     
- Misses         17       23       +6

Impacted Files	Coverage Δ
utils/middlewares/service.ts	`85.71% <66.66%> (-14.29%)`	⬇️
UploadServiceLogo/handler.ts	`82.75% <82.75%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cd66db...cc0f226. Read the comment docs.

gunzip · 2020-02-10T11:41:32Z

UploadServiceLogo/handler.ts

+    }
+
+    // tslint:disable-next-line:no-object-mutation
+    context.bindings.logo = Buffer.from(logoPayload.logo, "base64");


can Buffer.from() fail ?

it looks like you have to wrap it around a tryCatch() block:
https://nodejs.org/api/buffer.html#buffer_class_method_buffer_from_array

It shouldn't be necessary, because according to the documentation a TypeError will be thrown if logoPayload.logo is not an appropriate type, but its type is already checked and validated by the LogoPayload middleware inside the middlewaresWrap.

gunzip · 2020-02-10T11:43:04Z

UploadServiceLogo/handler.ts

+>;
+
+export function UpdateServiceLogoHandler(
+  _GCTC: CustomTelemetryClientFactory,


it looks like the telemetryclient is unused

gunzip · 2020-02-10T11:44:04Z

UploadServiceLogo/index.ts

+const cosmosDbUri = getRequiredStringEnv("CUSTOMCONNSTR_COSMOSDB_URI");
+const cosmosDbKey = getRequiredStringEnv("CUSTOMCONNSTR_COSMOSDB_KEY");
+const cosmosDbName = getRequiredStringEnv("COSMOSDB_NAME");
+const logosHost = getRequiredStringEnv("LOGOS_HOST");


Suggested change

const logosHost = getRequiredStringEnv("LOGOS_HOST");

const logosHost = getRequiredStringEnv("SERVICE_LOGOS_HOST");

we must document this variable and the logos storage connection string into the readme (I know that it is empty atm)

gunzip · 2020-02-10T11:58:48Z

CreateService/index.ts

-}
-
-export default httpStart;
+import { Context } from "@azure/functions";


it looks like these files differs only for newlines, may you revert them ?

I'm sorry, I tried to revert, but I couldn't manage. 😕

can you try removing the file and checkin it out again ? (or copyingit from another clone)

I tried to copy it from another clone, also changing the core.autocrlf git setting for the project, but I couldn't manage to revert the line endings anyway... 😕

gunzip · 2020-02-10T15:56:46Z

CreateService/index.ts

-}
-
-export default httpStart;
+import { Context } from "@azure/functions";


can you try removing the file and checkin it out again ? (or copyingit from another clone)

gunzip · 2020-02-10T15:57:26Z

README.md

+| Variable name                          | Description                                                                       | type    |
+| -------------------------------------- | --------------------------------------------------------------------------------- | ------- |
+| SERVICE_LOGOS_HOST                     | The host name of the service logos storage                                        | string  |
+| StorageConnection                      | The connectiong string used to connect to Azure Storage services                  | string  |


is this is for the services logs I'd use a more explicit name as 'LogosStorageConnection'

gunzip · 2020-02-11T14:00:05Z

UploadServiceLogo/handler.ts

+
+    return ResponseSuccessRedirectToResource(
+      {},
+      `${logosHost}/services/${serviceId}.png`,


naming: if this embeds https prefix then it's logosUrl (not an hostname), same states for the name of the environment variable

alexgpeppe added 19 commits January 24, 2020 15:17

[#170127598] Add service model code from io-function-commons

b2bcb5c

[#170127598] Replace service model imports from io-functions-commons

cf1beb8

[#170127598] Add properties from io-metadata-services to service model

5e9caa5

[#170127598] Import service definition from io-function-commons specs

04ab292

[#170127598] Replace service attributes imports from io-functions-com…

f5f4e5c

…mons

[#170127598] Add ServiceMetadata props to toBaseType() return value

983f2ff

[#170127598] Refactor service metadata model properties to camelCase

64c9e6a

[#170127598] Add props from io-services-metadata definitions to specs

d69212e

[#170127598] Remove t.exact() from ServiceMetadata codec definition

ee2af5a

[#170127598] Rename scopeEnum into ScopeEnum in service model file

5c412ed

[#170127598] Add ServiceMetadata props to apiServiceToService() retur…

7588f62

…n value

[#170127598] Add ServiceMetadata props to retrievedServiceToApiServic…

82c0ce2

…e() return value

[#170127598] Import tests for service model from io-functions-commons

31689a3

[#170127598] Generate definitions from specs before running tests

485787b

[#170127598] Mock dates in test suites

892fc04

[#170333527] Add function to upload service logo

ab166e3

[#170333527] Add test suite for upload service logo handler

024bd65

[#170333527] Remove wrong schema property in specs file

038d56e

[#170333527] Fix lint errors

5366456

alexgpeppe requested review from cloudify and francescopersico as code owners February 4, 2020 14:28

alexgpeppe requested a review from gunzip February 4, 2020 14:28

[#170333527] Restore service model usage from io-functions-commons

ee3cc68

alexgpeppe changed the base branch from 170127598-extended-service-model to master February 10, 2020 11:25

gunzip reviewed Feb 10, 2020

View reviewed changes

alexgpeppe added 2 commits February 10, 2020 14:57

[#170333527] Remove telemetryclient from UpdateServiceLogo function

548b85b

[#170333527] Add to README.md a section about required env variables

d2722d5

[#170333527] Remove unused constant in UploadServiceLogo/index.ts

f7c195c

alexgpeppe requested a review from gunzip February 10, 2020 15:50

gunzip requested changes Feb 10, 2020

View reviewed changes

alexgpeppe added 4 commits February 10, 2020 17:09

[#170333527] Add comments to force file changes detection by git

c0d660b

[#170333527] Remove comments added in the previous commit

2aaa9d1

[#170333527] Fix lint error in GetService/index.ts file

917000e

[#170333527] Rename env var: StorageConnection -> LogosStorageConnection

7db42b7

gunzip requested changes Feb 11, 2020

View reviewed changes

[#170333527] Rename variable: logosHost -> logosUrl

cc0f226

gunzip approved these changes Feb 11, 2020

View reviewed changes

gunzip merged commit 083b8c7 into master Feb 11, 2020

gunzip deleted the 170333527-logo-upload branch February 11, 2020 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#170333527] Service logo upload function #10

[#170333527] Service logo upload function #10

alexgpeppe commented Feb 4, 2020

digitalcitizenship commented Feb 10, 2020 •

edited

Loading

Installation

Summary

Details

Additional Details

Credits

codecov-io commented Feb 10, 2020 •

edited

Loading

gunzip Feb 10, 2020

gunzip Feb 10, 2020

alexgpeppe Feb 10, 2020

gunzip Feb 10, 2020

gunzip Feb 10, 2020

gunzip Feb 10, 2020

gunzip Feb 10, 2020

alexgpeppe Feb 10, 2020

gunzip Feb 10, 2020

alexgpeppe Feb 10, 2020

gunzip Feb 10, 2020

gunzip Feb 10, 2020

gunzip Feb 11, 2020

	const logosHost = getRequiredStringEnv("LOGOS_HOST");
	const logosHost = getRequiredStringEnv("SERVICE_LOGOS_HOST");

[#170333527] Service logo upload function #10

[#170333527] Service logo upload function #10

Conversation

alexgpeppe commented Feb 4, 2020

digitalcitizenship commented Feb 10, 2020 • edited Loading

Affected stories

Installation

Summary

Details

Additional Details

Credits

codecov-io commented Feb 10, 2020 • edited Loading

Codecov Report

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

digitalcitizenship commented Feb 10, 2020 •

edited

Loading

codecov-io commented Feb 10, 2020 •

edited

Loading