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

Get Reports for Organization details feature in Aggregator #143 #177

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amirhossein18121380
Copy link

This pull request implements the Get Reports for Organization details feature in Aggregator as per issue #143.

Changes:
Implemented the Get Reports for Organization details feature in Aggregator.
Added the necessary code to retrieve the reports for an organization =>
(GetOrganizationReportById feature in owner.read.api)

Testing:
Unfortunately, I was unable to write unit tests for the GetOrganizationReportByIdHandler due to difficulties in handling the injection of clients. I would appreciate any guidance on how to properly test this handler.

Notes:
This feature is still in development and may require further testing and refinement before it is considered complete.
I would be happy to help with testing and refining this feature further.

@hamed-shirbandi
Copy link
Owner

Hey @amirhossein18121380, thanks for your contribution.

Regarding the test design, I decided not to test the application's read side on the unit level. It’s less important than the write side, and there are not many rules you need to protect against changes. It’s enough if you can protect your handler with at least one integration test.

Please ask for a review when you are done with your changes, and don’t hesitate to reach out if you have any questions or need me to discuss things in more detail.

Your contribution is always welcome.

@amirhossein18121380
Copy link
Author

amirhossein18121380 commented Jun 6, 2024

Hey @hamed-shirbandi ,
thanks for the feedback and guidance.
I was able to run one integration test successfully.
Thanks again for your help!

Could you please let me know if there are any issues or feedback you'd like me to address?

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.

2 participants