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

Refactor/135-Code check: Database Results #140

Merged
merged 31 commits into from
May 24, 2024

Conversation

thangixd
Copy link
Collaborator

@thangixd thangixd commented May 23, 2024

Hi,

this pull request introduces a series of refactoring changes aimed at improving the maintainability and readability of the Database Results application. Significant updates have been made to JavaScript functionality, HTML structure, and the django app.

Changes Made:

JavaScript Updates:
Refactored filter_toggle.js to include a CheckboxSelector class, eliminating the need for hardcoded checkbox handling.
Updated metrics_dashboards.js to utilize constants for configuration while removing magic numbers and add helper function for setting tile colors, enhancing code consistency.

HTML Structure:

Reformatted all HTML files to ensure code consistency and readability.

Django views Refactoring:
Improved the get_context_data method in metrics_dashboard by segmenting the data retrieval process into multiple functions, enhancing readability and maintainability.
Similarly refactored get_context_data in EvaluationView to use smaller, purpose-specific functions for data processing.
Added comprehensive docstrings to every module in views.py, improving documentation and code understandability.
Typing in most modules

This PR closes #135.

Please check the changes and provide feedback. Also check the doc strings. I think it's best if @soeren227 and @tkv29 review the functionality and docs. Code review can be done by someone else.

@thangixd thangixd self-assigned this May 23, 2024
@thangixd thangixd changed the title Code check: Database Results Refactor/135-Code check: Database Results May 23, 2024
@tkv29
Copy link
Collaborator

tkv29 commented May 23, 2024

Did u also implemented @nils-schmitt comment
image

@tkv29
Copy link
Collaborator

tkv29 commented May 23, 2024

@soeren227 could you check the functionality of the evaluation view pls

@thangixd
Copy link
Collaborator Author

Did u also implemented @nils-schmitt comment image

yes

@soeren227
Copy link
Collaborator

@soeren227 could you check the functionality of the evaluation view pls

i did and everything works fine for me

@tkv29 tkv29 requested review from soeren227 and PitButtchereit May 23, 2024 18:07
@nils-schmitt
Copy link
Collaborator

@thangixd when you reload the branch with clean database, are all mirgations applied? If not, please do so and push again

…most_frequent_timestamp_correctness_count in Timestamp Correctness
@thangixd
Copy link
Collaborator Author

@thangixd when you reload the branch with clean database, are all mirgations applied? If not, please do so and push again

should be already migrated

Copy link
Collaborator

@nils-schmitt nils-schmitt left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@soeren227 soeren227 left a comment

Choose a reason for hiding this comment

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

Everything looks good. Just one suggestion for rephrasing a docstring and one question concerning formatting.
GJ!

@thangixd thangixd merged commit 89334d7 into main May 24, 2024
2 checks passed
@thangixd thangixd deleted the refactor/135-refactor-db-results branch May 24, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code check: Database Results
4 participants