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

[#IP-184] Add payee management to Send Message #158

Merged
merged 2 commits into from
Oct 12, 2021

Conversation

AleDore
Copy link
Contributor

@AleDore AleDore commented Sep 28, 2021

List of Changes

  • Upgrade io-functions-commons ref with new Message model and Payment Data definition
  • Upgrade @pagopa/ts-commons with new version that allows to accept infinite middlewares
  • Add Middleware to ensure only Users with ApiMessageWriteWithPayee can send messages with defined payee
  • Add override logic for payment messages in StoreMessageContentActivity
  • Update openapi specs

Motivation and Context

This PR allows to Send Payment messages with specific payee only for users (and services) that are authorized to.
In addition we check over payment_data if present and override payee value with default sender fiscal_code value for other users/services.

How Has This Been Tested?

unit test
Io-mock

Screenshots (if appropriate):

Types of changes

  • Chore (nothing changes by a user perspective)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@pagopa-github-bot
Copy link
Contributor

pagopa-github-bot commented Sep 28, 2021

Warnings
⚠️ This PR changes a total of 411 LOCs, that is more than a reasonable size of 250. Consider splitting the pull request into smaller ones.
⚠️ Please include a Pivotal story at the beginning of the PR title (see below).

Example of PR titles that include pivotal stories:

  • single story: [#123456] my PR title
  • multiple stories: [#123456,#123457,#123458] my PR title

Generated by 🚫 dangerJS against ddb2833

@AleDore AleDore marked this pull request as ready for review September 28, 2021 16:20
@AleDore AleDore requested a review from a team as a code owner September 28, 2021 16:20
Copy link
Contributor

@michaeldisaro michaeldisaro left a comment

Choose a reason for hiding this comment

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

LGTM, apart knowing why node downgrade is necessary.

@@ -1 +1 @@
14.17.5
14.16.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this downgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this is the version supported by Azure :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I imagined that. 👍

@michaeldisaro
Copy link
Contributor

It's two days that danger is failing with that error, should we worry about it?

@AleDore AleDore changed the title [IP-184] Add payee management to Send Message [#IP-184] Add payee management to Send Message Sep 29, 2021
@AleDore
Copy link
Contributor Author

AleDore commented Sep 29, 2021

It's two days that danger is failing with that error, should we worry about it?

We have to refactor danger plugin with Jira integration pagopa/danger-plugin-digitalcitizenship#114

@michaeldisaro
Copy link
Contributor

It's two days that danger is failing with that error, should we worry about it?

We have to refactor danger plugin with Jira integration pagopa/danger-plugin-digitalcitizenship#114

I think we also have to remove this "pivotal" warning.

@AleDore AleDore merged commit 0d2ab08 into master Oct 12, 2021
@AleDore AleDore deleted the IP-184_OpenApi_send_message_specs_with_payee branch October 12, 2021 14:35
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