fix: pass issue context through template context instead of ad-hoc model attributes#971
fix: pass issue context through template context instead of ad-hoc model attributes#971abdella-h wants to merge 3 commits intoNixOS:mainfrom
Conversation
…stead of attaching ad-hoc attributes to NixpkgsIssue
florentc
left a comment
There was a problem hiding this comment.
Thank you for your contribution. You are very close to the pattern we use for most of the suggestion-related templates, so as a step towards more consistency could you please go one step further and fit the pattern in question? By this I mean define an IssueContext type and have the issue component take only one parameter, data of type IssueContext, and then use the various data.this or data.that in the issue template.
|
@florentc Thanks for the feedback. I've updated the PR to use an IssueContext type following the same pattern as SuggestionContext |
| # Issues | ||
|
|
||
|
|
||
| @dataclass | ||
| class IssueContext: | ||
| issue: NixpkgsIssue | ||
| suggestion_context: SuggestionContext | ||
| github_issue: str | None |
There was a problem hiding this comment.
This belongs alongside the issue view. Please either move the issue view from the global views.py to its specific directory like for suggestions, or just define IssueContext alongside the view. You defined the type with the suggestions here.
There was a problem hiding this comment.
I moved IssueContext to views.py
| {% for data in issue_contexts %} | ||
| {% issue data show_permalink=True %} |
There was a problem hiding this comment.
| {% for data in issue_contexts %} | |
| {% issue data show_permalink=True %} | |
| {% for issue_context in issue_contexts %} | |
| {% issue issue_context %} |
Please move show_permalink to the issue_context too
| ) -> dict: | ||
| return { | ||
| "issue": issue, | ||
| "data": data, |
There was a problem hiding this comment.
Please move show_permalink to the issue_context too (this should just have data as a parameter in the end, and set it in the view
There was a problem hiding this comment.
The IssueContext now includes show_permalink, and the {% issue %} tag only takes data as a parameter
|
Could you solve the build issues? |
897fee2 to
c72a324
Compare
Problem
get_context_data()attachedsuggestion_contextandgithub_issuedirectly ontoNixpkgsIssuemodel instances — fields that don't exist on the model. This only worked accidentally because Django templates useobject.__dict__internally.Fix
Build an explicit
issue_contextslist of dictionaries and pass it through the template context properly. Updateissue_list.htmltoiterate over
issue_contextsinstead ofobject_list.