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

[#167569704] Add GetLimitedProfile function #6

Merged
merged 2 commits into from
Aug 20, 2019

Conversation

francescopersico
Copy link
Contributor

Draft because we need to decide how to handle the routes of the two http triggered functions.

For GetLimitedProfile i have set v1/profiles/{fiscalCode}. If it is ok for you i can change the CreateMessage route from {*segments} to v1/messages/{fiscalCode}.


/**
* Whether the sender service is allowed to send
* notififications to the user identified by this profile
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* notififications to the user identified by this profile
* messages to the user identified by this profile

}

/**
* Converts the Profile model to LimetedProfile type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Converts the Profile model to LimetedProfile type.
* Converts the Profile model to LimitedProfile type.


const middlewaresWrap = withRequestMiddlewares(
AzureApiAuthMiddleware(
new Set([UserGroup.ApiLimitedProfileRead, UserGroup.ApiFullProfileRead])
Copy link
Contributor

Choose a reason for hiding this comment

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

since this API will be used only by 3rd party services, we can limit the groups to ApiLimitedProfileRead (ApiFullProfileRead is only used by the app backend)

Suggested change
new Set([UserGroup.ApiLimitedProfileRead, UserGroup.ApiFullProfileRead])
new Set([UserGroup.ApiLimitedProfileRead])

} from "../handler";

describe("isSenderAllowed", () => {
it("should return false is the service is not allowed to send notifications to the user", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it("should return false is the service is not allowed to send notifications to the user", () => {
it("should return false if the service is not allowed to send notifications to the user", () => {

expect(isAllowed).toBe(false);
});

it("should return true is the service is allowed to send notifications to the user", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it("should return true is the service is allowed to send notifications to the user", () => {
it("should return true if the service is allowed to send notifications to the user", () => {

@cloudify
Copy link
Contributor

Draft because we need to decide how to handle the routes of the two http triggered functions.

For GetLimitedProfile i have set v1/profiles/{fiscalCode}. If it is ok for you i can change the CreateMessage route from {*segments} to v1/messages/{fiscalCode}.

yes go ahead, thanks

@francescopersico francescopersico force-pushed the 167569704-getlimitedprofile branch from df80a3c to ba354b0 Compare August 20, 2019 11:03
@digitalcitizenship
Copy link

Affected stories

  • 🌟 #167569704: Creare in io-functions-services la function GetLimitedProfile

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented Aug 20, 2019

Codecov Report

Merging #6 into master will increase coverage by 3.35%.
The diff coverage is 88.63%.

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
+ Coverage   73.33%   76.68%   +3.35%     
==========================================
  Files           2        3       +1     
  Lines         150      193      +43     
  Branches       14       19       +5     
==========================================
+ Hits          110      148      +38     
- Misses         40       45       +5

@francescopersico francescopersico marked this pull request as ready for review August 20, 2019 11:06
@cloudify cloudify merged commit 1480157 into master Aug 20, 2019
@gunzip gunzip deleted the 167569704-getlimitedprofile branch April 18, 2020 23:02
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.

3 participants