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

feat: noticed integration (Notification Page) #3243

Merged
merged 42 commits into from Aug 16, 2022

Conversation

vedant-jain03
Copy link
Member

@vedant-jain03 vedant-jain03 commented Jul 21, 2022

Fixes #2321

Describe the changes you have made in this PR -

  • Added noticed gem.
  • Config DB, Routes, Model.
  • Added notification for:
    - Star a Project.
    - Fork a Project.
  • UI
  • Activity_notification Remove

UI

  • Notification Page
    Screenshot from 2022-07-29 22-09-37

  • Navbar Quick Notifications
    Screenshot from 2022-08-10 10-56-31

Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.

@vedant-jain03 vedant-jain03 self-assigned this Jul 21, 2022
@vedant-jain03 vedant-jain03 added 🌟 feature work-in-progress GSoC'22 Label for Google Summer of Code 2022 PR's and Issues related. labels Jul 21, 2022
@vedant-jain03 vedant-jain03 added this to the Platform Improvement milestone Jul 21, 2022
@vedant-jain03 vedant-jain03 changed the title Noticed integration feat: noticed integration (Notification Page) Jul 21, 2022
app/notifications/fork_notification.rb Show resolved Hide resolved
app/notifications/star_notification.rb Show resolved Hide resolved
app/models/notification.rb Outdated Show resolved Hide resolved
app/models/star.rb Outdated Show resolved Hide resolved
app/notifications/fork_notification.rb Outdated Show resolved Hide resolved
app/notifications/star_notification.rb Outdated Show resolved Hide resolved
spec/factories/notifications.rb Outdated Show resolved Hide resolved
spec/models/notification_spec.rb Outdated Show resolved Hide resolved
spec/models/notification_spec.rb Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jul 21, 2022

Coverage Status

Coverage increased (+0.06%) to 81.907% when pulling d7ebfa0 on vedant-jain03:noticed_integration into 77045cc on CircuitVerse:master.

app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
padding-bottom: 5px;
}
.tab_container .active {
font-weight: bold;
Copy link

Choose a reason for hiding this comment

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

Properties should be ordered border-bottom, font-weight

app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
@@ -95,6 +97,10 @@ def fork(user)
forked_project.update!(
view: 1, author_id: user.id, forked_project_id: id, name: name
)
@project = Project.find(id)
if @project.author != user
Copy link

Choose a reason for hiding this comment

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

Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.

app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/users.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/navbar.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/navbar.scss Outdated Show resolved Hide resolved
end

def mark_all_as_read
NoticedNotification.where(recipient: current_user, read_at: nil).update_all(read_at: Time.zone.now)
Copy link

Choose a reason for hiding this comment

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

Avoid using update_all because it skips validations.

@codeclimate
Copy link

codeclimate bot commented Aug 16, 2022

Code Climate has analyzed commit d7ebfa0 and detected 7 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2
Bug Risk 2
Style 3

View more on Code Climate.

@tachyons tachyons merged commit 55ddeac into CircuitVerse:master Aug 16, 2022
@vedant-jain03
Copy link
Member Author

Finally 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 feature GSoC'22 Label for Google Summer of Code 2022 PR's and Issues related. work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve notifications page
3 participants