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

added status of comment #7375

Closed
wants to merge 4 commits into from
Closed

added status of comment #7375

wants to merge 4 commits into from

Conversation

keshavsethi
Copy link
Member

Fixes #7362
here status is showing as 1 and 4 as shown
Screenshot from 2020-01-28 23-52-21
Screenshot from 2020-01-28 23-52-26

Thanks!

@keshavsethi keshavsethi requested a review from a team as a code owner January 28, 2020 18:29
@codecov
Copy link

codecov bot commented Jan 28, 2020 •
edited
Loading

Codecov Report

Merging #7375 into master will decrease coverage by 4.94%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7375      +/-   ##
==========================================
- Coverage   81.86%   76.92%   -4.95%     
==========================================
  Files          97       97              
  Lines        5602     5715     +113     
==========================================
- Hits         4586     4396     -190     
- Misses       1016     1319     +303
Impacted Files Coverage Δ
app/controllers/talk_controller.rb 100.00% <0.00%> (ø) ⬆️
app/mailers/password_reset_mailer.rb 100.00% <0.00%> (ø) ⬆️
app/channels/application_cable/channel.rb 100.00% <0.00%> (ø) ⬆️
app/models/drupal_content_field_mapper.rb 100.00% <0.00%> (ø) ⬆️
app/channels/application_cable/connection.rb 100.00% <0.00%> (ø) ⬆️
app/models/drupal_content_field_map_editor.rb 100.00% <0.00%> (ø) ⬆️
app/controllers/home_controller.rb 91.93% <0.00%> (-6.46%) ⬇️
app/controllers/search_controller.rb 97.67% <0.00%> (ø) ⬆️
app/models/spamaway.rb 92.30% <0.00%> (-2.57%) ⬇️
app/services/execute_search.rb 88.88% <0.00%> (-5.56%) ⬇️
... and 49 more

@cesswairimu
Copy link
Collaborator

Looks good @keshavsethi 👍 I am just wondering if the numbers will be of help or rather @jywarren How do moderators know what each status means? Thanks

@SidharthBansal
Copy link
Member

@cesswairimu refer https://github.com/publiclab/plots2/blob/master/doc/DATA_MODEL.md#Features please

Uzay-G
Uzay-G reviewed Jan 30, 2020
View reviewed changes
app/views/comments/_comments.html.erb Outdated
<a class="btn btn-outline-secondary btn-sm" href="/admin/publish_comment/<%= comment.id %>"><%= translation('dashboard.moderate.approve') %></a>
<a class="btn btn-outline-secondary btn-sm" href="/admin/mark_comment_spam/<%= comment.id %>"><%= translation('dashboard.moderate.spam') %></a>
<% elsif current_user &. can_moderate? %>
Status:<%= comment.status %>
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a space here between status: and the number.
Also for the same part that exists above. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

@Uzay-G Sure, i will add this

@cesswairimu
Copy link
Collaborator

cesswairimu commented Jan 31, 2020 •
edited
Loading

Thanks @SidharthBansal, I meant it might be helpful to have the status in works displayed on the ui...do all moderators know what these status mean? But I guess we can follow that up with another issue if needed?

@SidharthBansal
Copy link
Member

SidharthBansal commented Jan 31, 2020 via email

@cesswairimu
Copy link
Collaborator

cesswairimu commented Feb 1, 2020 •
edited
Loading

Cool,
@keshavsethi, what we can do here..is create a method in comment model something like

def status_value
    if status == 0
      'banned'
    elsif status == 1
      'normal'
    elsif status == 4
      'moderated'
    else
      status
    end
  end

Then on the view call comment.status_value. How does this sound?

@keshavsethi
Copy link
Member Author

@cesswairimu Sure, I will do this

@keshavsethi
Copy link
Member Author

Screenshot from 2020-02-02 20-13-56
@cesswairimu @SidharthBansal @Uzay-G status message is added here, please review this
Thanks

cesswairimu
cesswairimu reviewed Feb 3, 2020
View reviewed changes
app/models/comment.rb Outdated
'Not Defined'
end
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks great, could you please remove this line and remove the space after 'Not defined' so that codeclimate check can pass? We should be ready to merge after that. Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

@cesswairimu @SidharthBansal Why some tests are not successful?
Do i need to change anything else?
Thanks

@cesswairimu
Copy link
Collaborator

restarting travis

sssash18
sssash18 suggested changes Feb 9, 2020
View reviewed changes
app/views/comments/_comments.html.erb Outdated
Comment on lines 41 to 43
<i class="fa fa-ban"></i>
<i class="fa fa-ban"></i>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the indented space as before.Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

@sssash18 Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

The tests failing doesn't seem relevant to your changes. I think you must close and reopen the PR to restart travis.

@SidharthBansal
Copy link
Member

Please open up a new PR and close this one.
Travis hurts many times

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Uzay-G Uzay-G Uzay-G left review comments

@sssash18 sssash18 sssash18 requested changes

@cesswairimu cesswairimu Awaiting requested review from cesswairimu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add "status" field to /comments page, viewable by admins/moderators only
5 participants
@keshavsethi @cesswairimu @SidharthBansal @Uzay-G @sssash18