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

Improve email generation logic #1032

Open
RichDom2185 opened this issue Nov 6, 2023 · 1 comment · May be fixed by #1070
Open

Improve email generation logic #1032

RichDom2185 opened this issue Nov 6, 2023 · 1 comment · May be fixed by #1070
Assignees

Comments

@RichDom2185
Copy link
Member

RichDom2185 commented Nov 6, 2023

As mentioned in #1031, I really think #932 is the wrong approach to generate the emails:

image

all_submissions_by_grader_for_index generates a view (JSON string), which is then converted back to a "model" using Jason.decode, which is then further manipulated, and the final view is generated using the email template.

This results in a lot of coupling, especially across the various layers of the backend, which means PRs like #1031 are unnecessarily breaking.

My proposal would be to create a separate function entirely for this functionality, which would optimise everything as well.

@RichDom2185 RichDom2185 self-assigned this Nov 6, 2023
@RichDom2185
Copy link
Member Author

I feel the ideal way would simply be to create a function all_ungraded_submissions_by_grader which has a much more optimised SQL query to only return the necessary fields for the email (especially since the second argument to all_ungraded_submissions_by_grader is always true for emails)

@GabrielCWT GabrielCWT self-assigned this Feb 24, 2024
@GabrielCWT GabrielCWT linked a pull request Feb 24, 2024 that will close this issue
@GabrielCWT GabrielCWT linked a pull request Feb 24, 2024 that will close this issue
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 a pull request may close this issue.

2 participants