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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update tag_controller.rb #8221
Update tag_controller.rb #8221
Conversation
replaced the instances of .count and .length with .size
Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. |
I've done my task.Thank you for giving me the oppurtunity to do this. can you rewiev the code and give some feedback to me ?. |
Codecov Report
@@ Coverage Diff @@
## main #8221 +/- ##
==========================================
+ Coverage 81.81% 81.87% +0.05%
==========================================
Files 100 101 +1
Lines 5846 5902 +56
==========================================
+ Hits 4783 4832 +49
- Misses 1063 1070 +7
|
thanks so much @yusufatalay for opening this 馃帀 could you please add the issue link to the issue this pull request fixes on your pull request on this line "Fixes issue_link" this will help other contributors while reviewing your changes. It also helps in automatically closing the issue when the pull request is merged. Thanks |
@yusufatalay please update the link to the issue you are solving. Thanks |
I am confused can you walk me through it ? |
Hey @yusufatalay , thanks for your contribution and welcome to publiclab community. It's very important to link your PR to the issue that it intends to resolve, as this can:
To link your PR to the issue, edit your comment (the first one, i.e. one with which you have opened this PR) and you'll see something like "Fixes #0000 (<=== Add issue number here)". This is even the first line of your comment. You have to replace 0000 here with the issue number concerned with your PR, and leave everything else as it is. Don't remove the # as it is crucial and creates the link in between. If you're still unsure and having problems, watch this as an example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yusufatalay , nice work. There're remaining instances of .count
still in
plots2/app/controllers/tag_controller.rb
Lines 545 to 565 in ce68627
def fetch_counts | |
# Enhancement #6306 - Add counts to `by type` dropdown on tag pages | |
@counts = {} | |
@counts[:posts] = Node.for_tagname_and_type(params[:id], 'note', wildcard: @wildcard).where('node.nid NOT IN (?)', @qids).count | |
@counts[:questions] = Node.for_tagname_and_type(params[:id], 'note', question: true, wildcard: @wildcard).where('node.nid IN (?)', @qids).count | |
@counts[:wiki] = Node.for_tagname_and_type(params[:id], 'page', wildcard: @wildcard).count | |
params[:counts] = @counts | |
# end Enhancement #6306 ============================================ | |
@total_posts = case @node_type | |
when 'note' | |
@notes.count | |
when 'questions' | |
@questions.count | |
when 'wiki' | |
@wikis.count | |
when 'maps' | |
@nodes.count | |
end | |
end | |
end |
and similarly I've seen that there are some remaining instances of .length
also in code.
TIP: Use Ctrl+F
after you've done editing and search for .count
and .length
keywords to see if there're any.
CAUTION: There're chances that similar keywords are present like counts
or even the count
word in some variable or string. You should not edit them. I think these would be rare, if you search with the .
in front.
Thanks, keep up the good work! 馃憤
Hey @yusufatalay , you've linked this PR with your PR, while you need to link the concerned issue to this PR. I checked and saw that the issue concerned with your PR is #8209. So, your comment should be like "Fixes #8209". Thanks, good work. 馃憤 |
Also, @cesswairimu , review this when you're free. |
Thank you for guiding me it really helps me improve myself. Those .counts must be added after I edit the code , if not please let me know. |
Hey @yusufatalay , these instances are not added after you've edited the code as these are also present in your fork https://github.com/yusufatalay/plots2/blob/patch-1/app/controllers/tag_controller.rb. Now, you should checkout to your Feel free to ping back if you're stuck or need any help. 馃憤 |
I figure it out that on github ctrl+f won't work for whole code. So I copied it to a text editor then I've edited it there thank you for your help , I've learned alot from this. |
@yusufatalay it's good to hear that you learnt a lot. Could you push your changes so that we can review it?
Thanks 馃憤 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all good now @yusufatalay . 馃帀 Thanks for the contribution.
@cesswairimu for review and merge!
@yusufatalay close #8360, as this PR is gonna solve the issue and would be merged hopefully! Thanks |
@yusufatalay don't close this PR, this is good and it's working. Also tests have passed. Reopen this PR. Close #8360, the new one that you've opened. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all good to me 馃帀
@cesswairimu check this one and merge when you're free!
Thank you for patiently teaching me. |
Really love the collaboration that has gone on here . Great job 馃帀 馃帀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
馃殌
Welcome to the community @yusufatalay 馃巿 |
Congrats on merging your first pull request! 馃檶馃帀鈿★笍 Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 馃檶 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 馃槙 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 馃槃 馃帀 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
Welcome to the community @yusufatalay 馃殌 and thanks @cesswairimu 馃槃 |
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
* Update tag_controller.rb replaced the instances of .count and .length with .size * Update tag_controller.rb
replaced the instances of .count and .length with .size
Fixes #8209
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!