Fixed username error #9435
Fixed username error #9435
Conversation
|
|
Codecov Report
@@ Coverage Diff @@
## main #9435 +/- ##
=======================================
Coverage ? 81.36%
=======================================
Files ? 98
Lines ? 5930
Branches ? 0
=======================================
Hits ? 4825
Misses ? 1105
Partials ? 0 |
There was a problem hiding this comment.
Looks good to me @gaurav2699 🎉
Please add a screenshot of the change before and after you made the changes 😅 .
|
Also, @gaurav2699 next time make sure to use a local branch instead of your |
|
Definitely @pydevsg, thanks a lot! |
|
@pydevsg I added screenshots. Thanks! |
|
Last but not least squash commits when you make so many commits. |
|
sure @pydevsg. I dont know why commits from my previous PR's that got merged are also here. Maybe because I didn't change the branch. I will squash it and make sure I use a different branch next time. Thanks |
|
Code Climate has analyzed commit 160a83d and detected 0 issues on this pull request. View more on Code Climate. |
There was a problem hiding this comment.
@gaurav2699 LGTM 🎉 For squashing the commits you may use git rebase -i HEAD~no.of.commits and then set as required.
|
Looking good 🎉 ...I think we need to confirm with @jywarren if we want to delete user's nodes once the user is deleted once we get a green light on that we are good to merge here. Thanks all |
There was a problem hiding this comment.
Thanks for working on this @gaurav2699, I do agree with Cess, we need confirmation on whether a user's node should be deleted once they are out of the system...I wonder how this would affect nodes with co-authors or nodes that are important in the system...perhaps we should have a system ID for a "user"?? That maybe replaces the node uid when a user is deleted...
|
Yes @RuthNjeri. I also now think its not a good idea to delete the nodes when the user who created the node is deleted because we might lose some important nodes due to this, and there might be nodes with multiple authors which should not be deleted. Having a system ID makes sense, so we can show it as deleted user or something else like Anonymous maybe. |
|
Lets see what @jywarren thinks of this and I will make changes according to that. |
|
Hi all, i appreciate this conversation so much. Great work identifying the issue which i now see is affecting not only #9417 but also #9527, confirmed that we have some revisions without user records. (44698, 69032 uids in the database). Agreed that we should not allow this to happen but also THANK YOU for triple checking on the policy of preserving data before approving and merging this. Great call. I'm not 100% sure either. I think we really almost NEVER delete a user, so it's interesting to me to try to figure out how this happened in the first place. For one, I see that https://publiclab.org/wiki/revisions/getting-started and https://publiclab.org/wiki/revisions/spectral-workbench-api both show missing authors, and both around 8 years ago. So I wonder if this is no longer a risk; it's not like we've seen any user deletion recently. With that in mind I think i approve this! I'll check once more and then merge it. Thanks again!!! |
| @@ -46,10 +46,10 @@ module Frequency | |||
| # validates_attachment_content_type :photo_file_name, :content_type => %w(image/jpeg image/jpg image/png) | |||
|
|
|||
| has_many :images, foreign_key: :uid | |||
| has_many :node, foreign_key: 'uid' | |||
| has_many :node, foreign_key: 'uid', dependent: :destroy | |||
There was a problem hiding this comment.
Confirmed that this will, when the user is destroyed, delete all the nodes and revisions:
https://guides.rubyonrails.org/association_basics.html#why-associations-questionmark
* Added translations in users/settings.html.erb * Fixed pagination issue * fixed pagination issue * Fixed pagination in wiki/stale too * Delete identifier.sqlite * fixed a minor issue * Fixed minor issue * Delete identifier.sqlite * Added functional test to check pagination * reduced no of new wikis for test * reduced 31 to 12 * fixed username error * fixed username error Co-authored-by: Cess <[email protected]>
* Added translations in users/settings.html.erb * Fixed pagination issue * fixed pagination issue * Fixed pagination in wiki/stale too * Delete identifier.sqlite * fixed a minor issue * Fixed minor issue * Delete identifier.sqlite * Added functional test to check pagination * reduced no of new wikis for test * reduced 31 to 12 * fixed username error * fixed username error Co-authored-by: Cess <[email protected]>
Fixes #9421 (<=== Add issue number here)]
Screenshots:
This wiki doesnt have a user associated
no user exists with uid 4
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test@publiclab/reviewersfor help, in a comment belowIf tests do fail, click on the red
Xto 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!