feat (outputmanagement): added output management integration#161
feat (outputmanagement): added output management integration#161Venkatakrishnan774 wants to merge 32 commits into
Conversation
|
|
|
Hi. Could you give a more detailed description of this new feature for future reference? Also, is necessary that the title follows commit convention, for exemple: |
Venkatakrishnan774
changed the title
Updating Local Branch
betinacosta
changed the title
| ) | ||
| """ | ||
|
|
||
| def __init__(self, service_client): |
There was a problem hiding this comment.
Missing type annotation for service_client
| return error_mapping.get(status_code) | ||
|
|
||
| @staticmethod | ||
| def _create_output_error_response(error_type, message) -> OutputResponse: |
There was a problem hiding this comment.
Missing type annotation for error_type and message
| def validate_email_parameters( | ||
| notification_template_key: str, | ||
| to: List[str], | ||
| business_document: dict, |
There was a problem hiding this comment.
business_document: Dict[str, Any]
| to=to_emails, | ||
| business_document=business_document, | ||
| cc=[cc_email] if cc_email else None, | ||
| template_language="en", |
There was a problem hiding this comment.
template_language will always be en? Or is something that we should ask for and have a default if not pass?
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def create_client( |
There was a problem hiding this comment.
Shouldn't this receive a DestinationCredentialConfig as well? And then, if this object is not passed, validate the other params? You can take a look in other modules, such as auditlog_ng, for an example and see if this can be applied here.
| frozen = True | ||
| str_strip_whitespace = True | ||
|
|
||
| def get_destination(self): |
There was a problem hiding this comment.
Missing return type annotation
| # SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Integration-style unit tests for output management module.""" |
There was a problem hiding this comment.
Integration tests should be in a different folder, and be proper integration test, if possible, and if not possible, a justification must be given. You can look destination module integration tests to see an example.
3 participants
Description
Output Management is a BTP shared service which has features like sending documents to external system in various channels like EDI/EMAIL/Print. As part of this, we are mainly concentrating on the email use case where one can send email via our framework along with attachments if necessary.
We are exposing capabilities via util methods of sending email as BTP destination based and via integrated MCP hub. When using the MCP Hub way, we are giving the capability to construct the required payload and invoke the util functions. When using the BTP destination way, we are giving the options to input the destination details and the payloads.
Related Issue
Closes #<issue_number>
(Link to the GitHub issue this PR addresses)
Type of Change
Please check the relevant option:
How to Test
Describe how reviewers can test your changes:
Checklist
Before submitting your PR, please review and check the following:
Breaking Changes
If this PR introduces breaking changes, please describe:
Additional Notes
Add any additional context, screenshots, or information that would help reviewers.