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.

Sign up for GitHub

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

Jump to bottom

Ensure the Affected column in Issues#index is updated when an affected node is renamed or merged #1273

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Open

Ensure the Affected column in Issues#index is updated when an affected node is renamed or merged #1273

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d5cd2be
prevent multiple eventListener binds on tab change
MattBudz Jun 17, 2024
9a25d19
navigate to tabbed pages using browser back button
MattBudz Jun 17, 2024
35bd387
update changelog
MattBudz Jun 17, 2024
c3fabbf
limit scope of eventLister to prevent N bindings
MattBudz Jun 18, 2024
3d735cb
navigate with `Turbolinks` vs `location.reload()`
MattBudz Jun 19, 2024
fdccc4e
add tab to history rather than replacing current history
MattBudz Jun 19, 2024
8625ec3
set data in pushState to load page with tab param
MattBudz Jun 24, 2024
dce4e53
ensure issues table shows renamed affected nodes
MattBudz Jun 26, 2024
246b668
Update CHANGELOG
MattBudz Jun 26, 2024
bec3f22
prevent multiple eventListener binds on tab change
MattBudz Jun 17, 2024
07c5afe
navigate to tabbed pages using browser back button
MattBudz Jun 17, 2024
9f845bd
limit scope of eventLister to prevent N bindings
MattBudz Jun 18, 2024
52022ee
navigate with `Turbolinks` vs `location.reload()`
MattBudz Jun 19, 2024
c998d1a
add tab to history rather than replacing current history
MattBudz Jun 19, 2024
4bff9da
set data in pushState to load page with tab param
MattBudz Jun 24, 2024
64652f9
fix rebase
MattBudz Jun 26, 2024
6c0e79d
don't store all the details of `affected` in the cache key
MattBudz Jun 27, 2024
d1c677d
Merge branch 'develop' into issues/show-updated-affected-list
MattBudz Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
- Liquid: Make project-level collections available for Liquid syntax
- Upgraded gems: nokogiri, rails, rexml
- Bugs fixes:
- [entity]:
- [future tense verb] [bug fix]
- Issues: Update Affected column after a node has been renamed or merged
- Bug tracker items:
- [item]
- New integrations:
Expand Down
4 changes: 2 additions & 2 deletions app/views/issues/_table.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<% cache ['issues-table', @all_columns, issues.map(&:id), @issues.map(&:updated_at).map(&:to_i).sort.last, @tags] do %>
<% cache ['issues-table', @all_columns, issues.map(&:affected), issues.map(&:id), @issues.map(&:updated_at).map(&:to_i).sort.last, @tags] do %>
<% table_attributes = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this change? If only 1 node is updated, then the whole table's cache busts. Maybe we can go with just the per-issue caching change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this change, the table doesn't show the correct affected column because the cache isn't busted. Do we need to cache the table if we are caching each row?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah looks like I'm mistaken and the inner fragment caching are preserved and not busted: https://guides.rubyonrails.org/caching_with_rails.html#russian-doll-caching

behavior: 'dradis-datatable',
'default-columns': @default_columns.to_json,
Expand All @@ -21,7 +21,7 @@
</thead>
<tbody>
<% issues.each do |issue| %>
<% cache [issue, @all_columns, issue.try(:affected_count), issue.try(:state), 'issues-table', @tags] do %>
<% cache [issue, @all_columns, issue.try(:affected), issue.try(:state), 'issues-table', @tags] do %>
<tr id="issue-<%= issue.id %>" data-tag-url="<%= project_issue_path(current_project, issue) %>">
MattBudz marked this conversation as resolved.
Show resolved Hide resolved
<td class="select-checkbox" data-behavior="select-checkbox"></td>
<% @all_columns.each do |column| %>
Expand Down
Loading