contributor count on tags page fixed #2055
Conversation
Generated by 🚫 Danger |
|
@jywarren please see if this solves the problem! |
app/controllers/tag_controller.rb
Outdated
| @@ -321,7 +321,7 @@ def contributors | |||
| .references(:term_data, :node_revisions) | |||
| .where('term_data.name = ?', params[:id]) | |||
| .order('node_revisions.timestamp DESC') | |||
| @users = @notes.collect(&:author).uniq | |||
| @users = DrupalUser.where(uid: Tag.contributors(@tagnames[0])) | |||
There was a problem hiding this comment.
Is it possible to use User.where(id: ... here instead, since we're trying to stop using DrupalUsers? Thank you! This otherwise looks fantastic.
There was a problem hiding this comment.
Hi, I guess we'd have to refactor the Tag.contributors method then, which currently gives array of uid (drupal_user id)!! Let's give it a try.
There was a problem hiding this comment.
@jywarren I guess we need to write a conversion function in user modal which fetches user_ids against drupal_user_ids. Then we could replace the DrupalUser with User in L324. Do you think this is the correct way, however it doesn't completely rules out DrupalUser.
The function would look something like (Would go in User Model):-
def self.user_ids(drupal_user_ids)
user_ids=[]
drupal_users = DrupalUser.include(:user).where(uid: drupal_user_ids)
drupal_users.each do |du|
user_ids<<du.user.id
end
end
This would greatly increase the overload time since a separate query is required to fetch a user from drupal_user. Do you think we should do this???
There was a problem hiding this comment.
So, we've synced User.id and DrupalUser.uid and you can treat one like the other -- we did a big database migration a while back to ensure this. Does that help?
There was a problem hiding this comment.
Oh cool!! That would just need a line change and then it would be good to go!!
Yes, that helped!
| @@ -52,8 +52,9 @@ def belongs_to(current_user, nid) | |||
| node_tag && node_tag.uid == current_user.uid || node_tag.node.uid == current_user.uid | |||
| end | |||
|
|
|||
| def self.contributor_count(tagname) | |||
| def self.contributors(tagname) | |||
|
I think this looks perfect. Did you want to add a simple unit test for |
|
Sure, I'll do! |
ViditChitkara
force-pushed
the
contributors
branch
from
8f73401
to
f1ca538
Compare
|
Done with the tests! |
app/models/tag.rb
Outdated
| @@ -62,6 +63,11 @@ def self.contributor_count(tagname) | |||
| uids+=n.revision.collect(&:uid) | |||
| end | |||
| uids = uids.uniq | |||
| uids | |||
There was a problem hiding this comment.
Ah, so, do you think we should return an ActiveRecord collection here, instead of just uids? Like, replace this line with User.find uids instead -- and then the line above (https://github.com/publiclab/plots2/pull/2055/files#diff-a8eb2425ebeb0814d010864bb014e815R326) would be simpler -- make sense?
There was a problem hiding this comment.
Yes, makes sense, will do!!
There was a problem hiding this comment.
@jywarren done with the suggested changes!
| @@ -68,7 +68,7 @@ | |||
|
|
|||
| <% end %> | |||
|
|
|||
| <% if @notes.nil? || @notes.length == 0 %> | |||
| <% if @users.nil? || @users.length == 0 %> | |||
There was a problem hiding this comment.
Found some unexpected behaviour when @notes.length was 0 but @users still existed hence modified the condition a bit!!
|
Super, is this ready to go, then? Thanks! |
|
Yes!!
…On Jan 23, 2018 6:04 AM, "Jeffrey Warren" ***@***.***> wrote:
Super, is this ready to go, then? Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2055 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AVkX0YYlgwdj2X6dAkW48nS1mZtgRbVVks5tNSkugaJpZM4Rlbn0>
.
|
|
Awesome! |
* contributor count on tags page fixed fixes publiclab#2042 * minor tweaks * changes * added unit tests for Tag.contributors * minor tweaks
fixes #2042
Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!
rake test:allPlease be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays. Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.
Thanks!