Skip to content

#15:feat:report generation as HTML#39

Open
Tony-Thawatchai wants to merge 3 commits intomainfrom
feature/report/HTML-report-generation
Open

#15:feat:report generation as HTML#39
Tony-Thawatchai wants to merge 3 commits intomainfrom
feature/report/HTML-report-generation

Conversation

@Tony-Thawatchai
Copy link

Description

Issue Link: #15

Description:

Add feature where report_generator.py generates HTML output to report/src/report_store/output.html from a sample data

Dependencies

  • os
  • Jinja2

Type of Change

  • [ x] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • run python at report/src/report_generation/report_generator.py
  • check the result at report/src/report_store/output.html

Checklist:

Before you submit your pull request, please make sure you have completed the following:

  • I have read the CONTRIBUTING document.
  • [x ] I have checked that my code adheres to the code style of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

Screenshots (if applicable)

Screenshot 2024-02-12 at 23 36 37

Additional Notes

Please feel free to leave your feedback. This is my first time coding in Python so any improvement would be great for me to learn.

@Tony-Thawatchai Tony-Thawatchai added the enhancement New feature or request label Feb 13, 2024
@Tony-Thawatchai Tony-Thawatchai added this to the 1. MVP milestone Feb 13, 2024
@alchuu00
Copy link
Contributor

Looks good to me @Tony-Thawatchai. Just one more little change. I recieved output that Nam is going to create.

   [
      {policy A : "True"},
      {policy B: "False"}
    ]

So if you could please update sample_data to match it. Appreciate it!

Comment on lines +1 to +4
<!-- HTML template to render result from report_generator.py -->

<!DOCTYPE html>
<html lang="en">
Copy link
Member

Choose a reason for hiding this comment

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

report store is a class where it can take a file and store it. it can mock the storage for now. i.e
report_store.store(report)

# with open(output_file, 'w') as f:
# f.write(html_table)

# print("Done!") No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

newline

</tbody>
</table>
</body>
</html> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

newline

</tbody>
</table>
</body>
</html> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

newline

Comment on lines +21 to +27
# Get the absolute path to the directory containing report_generator.py
dir_path = os.path.dirname(os.path.realpath(__file__))

# Use this path to set the correct path to the template/ directory
template_dir = os.path.join(dir_path, 'template')


Copy link
Member

Choose a reason for hiding this comment

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

these should be encapsulated into its own class and it should invoke the policy class. i think alja had some pseudo code in the readme whcih demonstrates how the report generation should be done. @alchuu00

@alchuu00
Copy link
Contributor

@Tony-Thawatchai this is how I was thinking of making report generation service which will connect all modules together

class ReportGenerationService:
    def __init__(self, report_generator, listing_store, report_store, policy_evaluator):
        self.report_generator = report_generator
        self.listing_store = listing_store
        self.report_store = report_store
        self.policy_evaluator = policy_evaluator
        self.evaluated_data_collection = {}

    def evaluate_and_generate_report(self, listing_id):
        data = self.listing_store.get_listing_data(listing_id)
        evaluated_data = self.policy_evaluator.evaluate(data)
        self.evaluated_data_collection[listing_id] = evaluated_data
        report = self.report_generator.generate_report(self.evaluated_data_collection)
        filename = f"report_{listing_id}.{self.report_generator.output_format}"
        self.report_store.save_report(report, filename)```
so report generator only generates report and then report store stores it  

@nam-m
Copy link
Contributor

nam-m commented Feb 20, 2024

Looks good to me @Tony-Thawatchai. Just one more little change. I recieved output that Nam is going to create.

   [
      {policy A : "True"},
      {policy B: "False"}
    ]

So if you could please update sample_data to match it. Appreciate it!

Just gonna correct myself here, it should be boolean True or False instead of strings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants