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

Move reports views to separate pages #5686

Closed
wants to merge 70 commits into from

Conversation

benhalpern
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

This moves the sections of internal/reports into their own pages. I think the performance gains we get will make it smoother for admins to sort, discover, and generally make them check this part of the site more often due to speed of fewer (sometimes slow) queries.

- Sending message ID to frontend
- Deleting Message
- Use pusher to delete message realtime
- User can delete or edit their own messages
Message id was not sent to receiver by pusher
…dit_messages

Feature/ability to delete and edit messages
…open messages and increasing message number count
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Jan 24, 2020
@benhalpern benhalpern changed the title Ben/move reports views to separate pages Move reports views to separate pages Jan 24, 2020
Copy link
Contributor

@rhymes rhymes left a comment

Choose a reason for hiding this comment

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

I think this can be split a bit further in separate actions and templates.

ransack(params[:q])
@feedback_messages = @q.result.page(params[:page] || 1).per(5)
@view = params[:view]
if @view == "new-articles"
Copy link
Contributor

Choose a reason for hiding this comment

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

if we split these view == in different routes we gain an easier code organization, so instead of /internals/reports?view=name it could be /internal/reports/name and each of the view has its own action method

Comment on lines +20 to +26
<% if @view == "new-articles" %>
<%= render "new_articles" %>
<% elsif @view == "suspicious-users" %>
<%= render "suspicious_users" %>
<% else %>
<%= render "reports" if @view.blank? %>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

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

these could just be rendered by the controller.

/internal/reports/new-articles
/internal/reports/suspicious-users
/internal/reports/feedback-messages or just /internal/reports

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

Looks like there are some commits that shouldn't be in here. Can we fix this either via rebase or cherry-picking?

@mstruve
Copy link
Contributor

mstruve commented May 12, 2020

Since this hasn't been active for a while and there are some conflicts I am going to close it. If we still want this feature lets resolve the conflicts, get the specs to run green, then reopen the PR.

@mstruve mstruve closed this May 12, 2020
@rhymes rhymes deleted the ben/move-reports-views-to-separate-pages branch May 13, 2020 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: unreviewed bot applied label for PR's with no review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants