Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#175374436] Allow lowercase transformation on png naming while uploading a Service image Logo #95

Merged
merged 6 commits into from
Oct 22, 2020

Conversation

AleDore
Copy link
Contributor

@AleDore AleDore commented Oct 21, 2020

This PR introduce lowercase transformation on blob name while uploading a service logo, due to wrong discovery by IO-APP that uses lowercase naming convention.
It also introduce the azure storage' sdk usage, because these kind of transformations (lowercase of the variables) are not allowed in azure bindings approach.

So changes can be resumed as follow:

  • Out binding deletion (blob type) on function.json;
  • Handler now use blobService, in order to write blob on AssetsStorage identified by AssetsStorageConnection config variable;
  • Config module now handle AssetsStorageConnection that was missing;
  • Tests refactoring due to implementation changes;

@pagopa-github-bot
Copy link
Contributor

pagopa-github-bot commented Oct 21, 2020

Affected stories

  • 🌟 #175374436: Come APP, voglio che le immagini relative ai loghi dei servizi, caricate tramite le API di onboarding programmatico, mantengano il naming con il lowercase dell' id servizio

Generated by 🚫 dangerJS

@codecov-io
Copy link

codecov-io commented Oct 21, 2020

Codecov Report

Merging #95 into master will increase coverage by 0.12%.
The diff coverage is 90.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   82.93%   83.06%   +0.12%     
==========================================
  Files          48       48              
  Lines        1600     1624      +24     
  Branches      126      126              
==========================================
+ Hits         1327     1349      +22     
- Misses        268      270       +2     
  Partials        5        5              
Impacted Files Coverage Δ
utils/config.ts 75.00% <ø> (ø)
UploadServiceLogo/handler.ts 87.75% <90.00%> (+3.75%) ⬆️
utils/random.ts 77.77% <100.00%> (ø)

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 8b3f6b7...115f308. Read the comment docs.

@@ -27,6 +28,8 @@ const servicesContainer = database.container(SERVICE_COLLECTION_NAME);

const serviceModel = new ServiceModel(servicesContainer);

const blobService = createBlobService(config.StorageConnection);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isnt this AssetsStorageConnection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is but it's not present in config module :) I'd rather add this variable.

@AleDore AleDore requested a review from gunzip October 21, 2020 15:36
Copy link
Contributor

@balanza balanza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me what changes have been introduced in this PR. The title and the related story both mention to keep the name lowercase, but then the while upload process has been revised.

tUpsertBlobFromObject(
blobService,
"services",
`${serviceId.toLowerCase()}.png`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd save this value in a variable as it's being used later in the code.

@AleDore
Copy link
Contributor Author

AleDore commented Oct 21, 2020

It's not clear to me what changes have been introduced in this PR. The title and the related story both mention to keep the name lowercase, but then the while upload process has been revised.

I've updated description :)

@AleDore AleDore requested a review from balanza October 21, 2020 16:55
@balanza
Copy link
Contributor

balanza commented Oct 21, 2020

Lgtm

@AleDore
Copy link
Contributor Author

AleDore commented Oct 21, 2020

Ok let's wait before merging this, due to integration tests execution.

@AleDore AleDore requested a review from balanza October 22, 2020 10:20
@AleDore AleDore merged commit 8ae4aef into master Oct 22, 2020
@AleDore AleDore deleted the 175374436_lowecase_on_uploadServiceLogo_output_naming branch October 22, 2020 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants