Skip to content

fix: pass issue context through template context instead of ad-hoc model attributes#971

Open
abdella-h wants to merge 3 commits intoNixOS:mainfrom
abdella-h:fix/ad-hoc-attributes
Open

fix: pass issue context through template context instead of ad-hoc model attributes#971
abdella-h wants to merge 3 commits intoNixOS:mainfrom
abdella-h:fix/ad-hoc-attributes

Conversation

@abdella-h
Copy link
Copy Markdown
Contributor

Problem

get_context_data() attached suggestion_context and github_issue directly onto NixpkgsIssue model instances — fields that don't exist on the model. This only worked accidentally because Django templates use object.__dict__ internally.

Fix

Build an explicit issue_contexts list of dictionaries and pass it through the template context properly. Update issue_list.html to
iterate over issue_contexts instead of object_list.

…stead of attaching ad-hoc attributes to NixpkgsIssue
Copy link
Copy Markdown
Member

@florentc florentc left a comment

Choose a reason for hiding this comment

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

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.

@abdella-h
Copy link
Copy Markdown
Contributor Author

@florentc Thanks for the feedback. I've updated the PR to use an IssueContext type following the same pattern as SuggestionContext

Comment on lines +275 to +282
# Issues


@dataclass
class IssueContext:
issue: NixpkgsIssue
suggestion_context: SuggestionContext
github_issue: str | None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@abdella-h abdella-h Apr 3, 2026

Choose a reason for hiding this comment

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

I moved IssueContext to views.py

Comment thread src/webview/templates/issue_list.html Outdated
Comment on lines +15 to +16
{% for data in issue_contexts %}
{% issue data show_permalink=True %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
{% 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,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The IssueContext now includes show_permalink, and the {% issue %} tag only takes data as a parameter

Comment thread src/webview/suggestions/context/types.py Fixed
@florentc
Copy link
Copy Markdown
Member

florentc commented Apr 8, 2026

Could you solve the build issues?

@abdella-h abdella-h force-pushed the fix/ad-hoc-attributes branch from 897fee2 to c72a324 Compare April 10, 2026 16:19
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.

3 participants