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

Issue 1284 highlight alerts #1352

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@Rodrigue-K
Copy link
Contributor

Rodrigue-K commented Dec 13, 2018

This commit fixes the #1284 issue by adding an unread-alert class for "RepositoryVulnerabilityAlert' to the notifications_helper. However I don't think querying the database from notification helper is the best solution. Would love some feedback/ideas on improving this solution.

def sidebar_filter_link(active:, param:, value:, count: nil, except: nil, link_class: nil, path_params: nil, title: nil) css_class = 'nav-item' css_class += ' active' if active css_class += ' unread-alert' if unread_alerts?(value)

def unread_alerts?(type) type == 'RepositoryVulnerabilityAlert' && Notification.reorder(nil). where(unread: true, subject_type: "RepositoryVulnerabilityAlert").count > 0 end

Rodrigue-K added some commits Dec 1, 2018

Fixes issue #1284 (vulnerability alert)
This commit highlights vulnerability alert filter link in the sidebar if
there are any unread vulnerability alert notifications.
content_tag :li, class: (active ? 'nav-item active' : 'nav-item'), title: title do
css_class = 'nav-item'
css_class += ' active' if active
css_class += ' unread-alert' if unread_alerts?(value)

This comment has been minimized.

@andrew

andrew Dec 17, 2018

Member

Perhaps here instead of making a database query we could add a css class to name each sidebar_filter_link, for example by using the param and value arguments so we get a class like type-RepositoryVulnerabilityAlert.

You could change line 237 to:

css_class += " #{param}-#{value}"
@BenJam

This comment has been minimized.

Copy link
Contributor

BenJam commented Dec 21, 2018

would love to see this in 👍

@BenJam

This comment has been minimized.

Copy link
Contributor

BenJam commented Jan 2, 2019

I'll pick this up if @Rodrigue-K is busy elsewhere :)

@Rodrigue-K

This comment has been minimized.

Copy link
Contributor

Rodrigue-K commented Jan 5, 2019

I'll pick this up if @Rodrigue-K is busy elsewhere :)

Thank you

@BenJam BenJam self-assigned this Jan 7, 2019

BenJam added a commit that referenced this pull request Jan 11, 2019

@BenJam BenJam removed their assignment Jan 11, 2019

@BenJam BenJam closed this Jan 14, 2019

andrew added a commit that referenced this pull request Jan 14, 2019

Highlight vulnerability alerts (#1451)
* Added intsruction to clone to local installation guide

* Fixes issue #1284 (vulnerability alert)

This commit highlights vulnerability alert filter link in the sidebar if
there are any unread vulnerability alert notifications.

* rough implementation of #1352
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment