Deprecate drupal user model #2857

Neidley · 2018-06-19T15:48:49Z

No description provided.

Neidley · 2018-06-19T15:54:25Z

To start what I'm doing is going through the pages of search results from https://github.com/publiclab/plots2/search?p=2&q=drupaluser&type=&utf8=%E2%9C%93
and replacing DrupalUser with User in all the /models/ files.

Neidley · 2018-06-21T11:38:49Z

I've now refractored DrupalUser to User and drupal_user to user from the search results.
Now onto getting tests to pass.

jywarren · 2018-06-21T19:13:12Z

OK, we've had a lot of changes merged in the past couple days. It looks like you may need to resolve a few conflicts here before moving forward. I'll keep an eye out to help you with the tests!

jywarren · 2018-06-23T17:34:25Z

Hmm, looks stalled! I'll try to restart it.

Neidley · 2018-06-23T19:25:56Z

Hey sorry I haven’t been able to work on this the last couple days

jywarren · 2018-06-26T18:35:20Z

No problem!

…

On Sat, Jun 23, 2018, 3:26 PM Andrew Neidley ***@***.***> wrote: Hey sorry I haven’t been able to work on this the last couple days — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#2857 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJzy9tnJR2AoB_SAdBfOXMhK3NYADks5t_pZFgaJpZM4Utxjq> .

jywarren · 2018-07-10T17:13:07Z

Hi, just checking if you've gotten stuck on this at all, or if I could help in any way? Thanks!

Neidley · 2018-07-11T07:08:13Z

Thank you for your patience!
I could definitely use some help...
here's part of the error message I'm getting when I try to run things locally with passenger start after going through and changing the variables mentioned in the issue instructions:

ANDREWs-MacBook-Pro:plots2 andrewneidley$ passenger start
=============== Phusion Passenger Standalone web server started ===============
PID file: /Users/andrewneidley/Desktop/Bloc/code/open-source/contributions/plots2/passenger.3000.pid
Log file: /Users/andrewneidley/Desktop/Bloc/code/open-source/contributions/plots2/log/passenger.3000.log
Environment: development
Accessible via: http://0.0.0.0:3000/

You can stop Phusion Passenger Standalone by pressing Ctrl-C.
Problems? Check https://www.phusionpassenger.com/library/admin/standalone/troubleshooting/
===============================================================================
App 16226 output: PassengerAgent(16233,0x7fffa77be380) malloc: *** mach_vm_map(size=1048576) failed (error code=268435459)
App 16226 output: PassengerAgent(16233,0x7fffa77be380) malloc: *** error: can't allocate region securely
App 16226 output: PassengerAgent(16233,0x7fffa77be380) malloc: *** set a breakpoint in malloc_error_break to debug
App 16226 output: PassengerAgent(16243,0x7fffa77be380) malloc: *** mach_vm_map(size=8388608) fa
App 16226 output: iled (error code=268435459)
App 16226 output: PassengerAgent(16243,0x7fffa77be380) malloc: *** error: can't allocate region securely
App 16226 output: PassengerAgent(16243,0x7fffa77be380) malloc: *** set a breakpoint in malloc_error_break to debug
App 16226 output: PassengerAgent(16245,0x7fffa77be380) malloc: *** mach_vm_map(size=8388608) failed (error code=268435459)
App 16226 output: PassengerAgent(16245,0x7fffa77be380) malloc: *** error: can't allocate region securely
App 16226 output: PassengerAgent(16245,0x7fffa77be380) malloc: *** set a breakpoint in malloc_error_break to debug
[ N 2018-07-11 02:03:06.8689 16210/T5 age/Cor/SecurityUpdateChecker.h:517 ]: Security update check: no update found (next check in 24 hours)
[ E 2018-07-11 02:03:06.8882 16210/T5 Crypto.cpp:990 ]: Removing Passenger private key from keychain failed: Write permissions error.. Please remove the private key from the certificate labeled Phusion Passenger Open Source in your keychain.
App 16226 output: Not running mysql today.
OpenIdAuthentication.store is nil. Using in-memory store.
Started HEAD "/" for 127.0.0.1 at 2018-07-11 02:03:12 -0500
Processing by HomeController#home as HTML
App 16253 output: /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/activerecord-5.0.6/lib/active_record/associations/builder/collection_association.rb:28: warning: already initialized constant TagNodeAssociationExtension
App 16253 output: /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/activerecord-5.0.6/lib/active_record/associations/builder/collection_association.rb:28: warning: previous definition of TagNodeAssociationExtension was here

jywarren · 2018-07-11T15:31:08Z

Although these look like errors, they don't seem to have stopped the app from running, that I can see. What do you see in your browser when you go to http://0.0.0.0:3000/ after starting this?

Awesome!

I wanted to note that perhaps @Souravirus would be able to advise you on this as he's tackled some really big changes over the past few weeks and is an expert in this sort of thing. If he has time he may be a good person to ask for help!

Neidley · 2018-07-22T07:20:22Z

Gonna try to tackle this again...

Neidley · 2018-07-22T07:35:58Z

@Souravirus, if you're out there and have time to look at this.

Here's what I'm seeing when I start the server:

and here's what I'm getting this when I run rails test -d

724 runs, 0 assertions, 0 failures, 724 errors, 0 skips

Failed tests:

/Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/railties-5.0.6/lib/rails/test_unit/reporter.rb:70:in `method': undefined method `test_report_redirect' for class `Minitest::Result' (NameError)
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/railties-5.0.6/lib/rails/test_unit/reporter.rb:70:in `format_rerun_snippet'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/railties-5.0.6/lib/rails/test_unit/reporter.rb:41:in `block in aggregated_results'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/railties-5.0.6/lib/rails/test_unit/reporter.rb:41:in `map'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/railties-5.0.6/lib/rails/test_unit/reporter.rb:41:in `aggregated_results'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/railties-5.0.6/lib/rails/test_unit/reporter.rb:37:in `report'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/minitest-5.11.3/lib/minitest.rb:808:in `each'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/minitest-5.11.3/lib/minitest.rb:808:in `report'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/minitest-5.11.3/lib/minitest.rb:141:in `run'
	from /Users/andrewneidley/.rvm/gems/ruby-2.4.1/gems/minitest-5.11.3/lib/minitest.rb:63:in `block in autorun'

Will try to get what I can working now that I have more free time 👍 I promise!
Thank you for your patience on this issue

grvsachdeva · 2018-07-22T09:20:42Z

Hi @Neidley , I guess you need to rebase your PR. Thanks!

Souravirus · 2018-07-22T16:54:49Z

First if you can then please resolve the conflicts. That would be great thank you.

Souravirus · 2018-07-22T16:55:48Z

Yeah like @Gauravano try rebasing your PR. Sorry for being late here. I have not seen that I was mentioned here.

jywarren · 2018-07-23T19:26:36Z

Hi, @Neidley - there's some guidance on rebasing here:

you may be able to skip step 2 if your master is still in sync with publiclab's master: https://publiclab.org/wiki/contributing-to-public-lab-software#Rewinding+the+master+branch

Thanks!

Neidley · 2018-07-23T21:17:44Z

@Gauravano @Souravirus
Cool I think I've rebased it successfully, now I'll try to tackle the merge conflicts

Souravirus · 2018-07-23T21:30:02Z

hmm now you can continue. Great work

Souravirus · 2018-07-23T21:33:33Z

But there is an error with rake db:setup command. Please see to it

Souravirus · 2018-07-23T21:36:44Z

app/models/user.rb

-      self.id = drupal_user.uid
+    if user.nil?
+      	    user = User.new(name: username,
+                                    pass: rand(100_000_000_000_000_000_000),


I guess here is the error with pass that is coming in the tests. I guess pass is an unknown attribute with User model. So adding pass attribute to User model might do the work.

yeah my app/models/user.rb looks like that.
I ran rake db:setup and rake db:migrate which now shows me this error on localhost:
couldn't find file 'noty/lib/noty.css' with type 'text/css'

Aha! This corresponds to this section, where a matching drupal_user is created:

plots2/app/models/user.rb

Lines 53 to 81 in bce1f1f

def create_drupal_user

self.bio ||= ''

if drupal_user.nil?

drupal_user = DrupalUser.new(name: username,

pass: rand(100_000_000_000_000_000_000),

mail: email,

mode: 0,

sort: 0,

threshold: 0,

theme: '',

signature: '',

signature_format: 0,

created: DateTime.now.to_i,

access: DateTime.now.to_i,

login: DateTime.now.to_i,

status: 1,

timezone: nil,

language: '',

picture: '',

init: '',

data: nil,

timezone_id: 0,

timezone_name: '')

drupal_user.save!

self.id = drupal_user.uid

else

self.id = DrupalUser.find_by(name: username).uid

end

end

But since we're deprecating drupal_user, we can delete that whole section -- we don't need to do it anymore. We can also remove the hook which triggers it:

plots2/app/models/user.rb

Line 44 in bce1f1f

def self.search(query)

Make sense?

I think so, let me delete those and see what happens

Hmm right @jywarren, I guess deleting would be better and the removal of hooks if not done would be detected by tests. So it would work fine. So @Neidley you can remove this function in my opinion.

Do I need to make changes locally, or are we able to make changes within this PR and see if it works from here?
(Still new to the capabilities/complexities of PRs) and if changes can be made by you guys here how do I pull them back to my local branch to run code locally?

You should do the changes locally and push it here and if there is any conflicts shown up in the PR. Then this means you have to rebase it as you have done today.

So I'm deleting the create_user function from plots2/app/models/user.rb? (that was previously create_drupal_user function) and see what happens? Don't users need to still be created?

jywarren · 2018-07-23T22:09:33Z

so in this we previously had 2 types of users, which we kept in sync - users and drupal users. Now we are cutting that back to just one, users. So this was one of the various functions that had been creating a matching drupal_user record for each user record. No longer needed!

…

On Mon, Jul 23, 2018 at 6:08 PM Andrew Neidley ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/models/user.rb <#2857 (comment)>: > - created: DateTime.now.to_i, - access: DateTime.now.to_i, - login: DateTime.now.to_i, - status: 1, - timezone: nil, - language: '', - picture: '', - init: '', - data: nil, - timezone_id: 0, - timezone_name: '') - drupal_user.save! - self.id = drupal_user.uid + if user.nil? + user = User.new(name: username, + pass: rand(100_000_000_000_000_000_000), So I'm deleting the create_user function from plots2/app/models/user.rb? (that was previously create_drupal_user function) and see what happens? Don't users need to still be created? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2857 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJyPG7l-ah_wsNMDJSdkczxeHJFaHks5uJkkugaJpZM4Utxjq> .

Neidley · 2018-07-23T22:18:32Z

Cool, I've removed that create and self.search(query) function...
which seems to have lowered the codeclimate issues to fix so that's cool :)
(I've never used codeclimate and trying to figure out what it wants...is that what I should be focusing on?)

jywarren · 2018-07-23T22:28:22Z

app/models/user.rb

@@ -41,45 +41,11 @@ class User < ActiveRecord::Base
  before_save :set_token
  after_destroy :destroy_user

-  def self.search(query)


Ah, this one we still needed, though. But the other deletion looks good!

gotcha, adding it back now.
I guess I also should remove
before_create :create_user , now that create_user is gone, right?

That's right if create_user used to be create_drupal_user, which i think is right. Thanks!!!

Neidley · 2018-07-24T00:41:24Z

@jywarren I'm Looking at these remaining code climate errors...

Class User has 32 methods (exceeds 20 allowed). Consider refactoring. x1
Method contribution_graph_making has a Cognitive Complexity of 9 (exceeds 5 allowed). Consider refactoring. x1
Possible unprotected redirect x1
Unescaped model attribute x7
Are you familiar with these issues? Are they necessary to fix?

Neidley · 2018-07-24T00:48:19Z

@jywarren And looking at the continuous-integration/travis-ci
looks like it doesn't like this:
/app/app/models/user.rb:99:in uid'` (?)

grvsachdeva · 2019-01-02T20:01:59Z

This profile is created 2 months ago - https://unstable.publiclab.org/profile/iragersh and is working properly. And, Liz's profile is also throwing 500 - https://unstable.publiclab.org/profile/liz. So, problem coming with old profiles

jywarren · 2019-01-03T01:11:44Z

I booted it locally but can't find the issue. Need to check logs on unstable. Hang on!

jywarren · 2019-01-03T01:12:55Z

OK:

[74adabf0-5094-4c24-a2f6-fe88dfd816bb] Completed 500 Internal Server Error in 1938ms (ActiveRecord: 864.2ms)
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]   
[74adabf0-5094-4c24-a2f6-fe88dfd816bb] ActionView::Template::Error (undefined method `comment_count' for #<Node:0x000055eb6c63d348>
Did you mean?  comments_count
               comments_count?
               comments_count=
               comment_change):
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     25:           <% if like.type == 'note' %>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     26:             by <a <% if @widget %>target="_blank"<% end %> href="/profile/<%= like.author.name %>"><%= like.author.name %></a>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     27:      <%= distance_of_time_in_words(like.created_at, Time.current, { include_seconds: false, scope:'datetime.time_ago_in_words' }) %>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     28:             | <a <% if @widget %>target="_blank"<% end %> href="<%= like.path %>#comments"><i style="color:#888;" class="fa fa-comment-o"></i> <%= like.comment_count %></a>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     29:           <% else %>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     30:             <%= t('notes._notes.last_edit_by') %> <a <% if @widget %>target="_blank"<% end %> href="/profile/<%= like.latest.author.name %>"><%= like.latest.author.name %></a>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]     31:      <%= distance_of_time_in_words(Time.at(like.latest.timestamp), Time.current, { include_seconds: false, scope: 'datetime.time_ago_in_words' }) %>
[74adabf0-5094-4c24-a2f6-fe88dfd816bb]   
[74adabf0-5094-4c24-a2f6-fe88dfd816bb] app/views/users/_likes.html.erb:28:in `block in _app_views_users__likes_html_erb___2438298131426513554_472348078:

jywarren · 2019-01-03T01:14:09Z

Weird because you'd think that'd be caught by tests?

jywarren · 2019-01-03T01:17:49Z

Do we need to add a like to a user in fixtures, before testing the profile page?

jywarren · 2019-01-03T01:19:10Z

OK, was able to reproduce locally.

jywarren · 2019-01-03T01:19:41Z

pushing to unstable again

jywarren · 2019-01-03T01:49:35Z

https://unstable.publiclab.org/profile/liz working now!

jywarren · 2019-01-03T03:16:50Z

Hmm, login modal from https://unstable.publiclab.org/profile/stevie doesn't work... but maybe it doesn't on stable either? That's right: https://stable.publiclab.org/profile/stevie also.

jywarren · 2019-01-03T03:19:47Z

The site generally seems to work! I think we can merge this!

SidharthBansal · 2019-01-03T17:51:14Z

Hmm, login modal from https://unstable.publiclab.org/profile/stevie doesn't work... but maybe it doesn't on stable either? That's right: https://stable.publiclab.org/profile/stevie also.

Issue is already reported and being worked upon #4462.
Thanks for checking Jeff.

grvsachdeva

Looks good to me 👍 🎉

jywarren · 2019-01-03T19:30:49Z

🎉 🎉 🎉 this is complete!!! Thanks @Neidley for getting it started, this was a long journey!!!

SidharthBansal · 2019-01-03T19:32:07Z

Awesome. Thanks @Neidley for helping Jeff in making our codebase better.

…

On Fri, Jan 4, 2019, 1:00 AM Jeffrey Warren ***@***.*** wrote: 🎉 🎉 🎉 this is complete!!! Thanks @Neidley <https://github.com/Neidley> for getting it started, this was a long journey!!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2857 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AUACQxu69Ogk8xaNxvD6xuIYHJcqZ_Jsks5u_lpsgaJpZM4Utxjq> .

Follow-up to #2857 once we confirm we have a backup and all is running smoothly.

* Drop DrupalUsers "users" table Follow-up to #2857 once we confirm we have a backup and all is running smoothly. * Update schema.rb.example

* Drop DrupalUsers "users" table Follow-up to publiclab#2857 once we confirm we have a backup and all is running smoothly. * Update schema.rb.example

* initial local repo commit * all model files refactored DrupalUser to User * all controllers files refactored DrupalUser to User * updated .gitignore, refractored DrupalUser to User in all doc/app/ html files and test/* files * refractored drupal_user to user in numerous files * trying to ignore changes from unneccessary files * remove create function and hook from models/user.rb * fix some code climate errors * Lots and lots of refactoring * Update _likes.html.erb * Update users_controller.rb * cleanup profile_user * more user.rb cleanup * Update _comment.html.erb

* Drop DrupalUsers "users" table Follow-up to publiclab#2857 once we confirm we have a backup and all is running smoothly. * Update schema.rb.example

Neidley mentioned this pull request Jun 19, 2018

Deprecate remaining old DrupalUser code for plain User model #2838

Closed

jywarren closed this Jun 23, 2018

jywarren reopened this Jun 23, 2018

ghost assigned jywarren Jun 23, 2018

ghost added the in progress label Jun 23, 2018

Neidley force-pushed the deprecate-drupal-user-model branch from 9fbadb9 to e1a9d87 Compare July 23, 2018 21:15

Souravirus reviewed Jul 23, 2018

View reviewed changes

jywarren reviewed Jul 23, 2018

View reviewed changes

Update _likes.html.erb

4c00c87

grvsachdeva approved these changes Jan 3, 2019

View reviewed changes

jywarren added 4 commits January 3, 2019 13:51

Update users_controller.rb

2f07cec

cleanup profile_user

b855a8d

more user.rb cleanup

8e4ccbf

Update _comment.html.erb

1d23f53

jywarren merged commit 258bf08 into publiclab:master Jan 3, 2019

ghost removed the review-me label Jan 3, 2019

jywarren added a commit that referenced this pull request Jan 3, 2019

Drop DrupalUsers "users" table

90e1b8f

Follow-up to #2857 once we confirm we have a backup and all is running smoothly.

This was referenced Jan 3, 2019

Drop DrupalUsers "users" table #4514

Merged

Refactor Stats Controller #4300

Merged

Refactoring profile.html.erb code pt.1 #4355

Closed

grvsachdeva mentioned this pull request Mar 12, 2019

user lookup by email doesn't check the old user table #154

Closed

jywarren added a commit that referenced this pull request Mar 31, 2019

Drop DrupalUsers "users" table (#4514)

251cd61

* Drop DrupalUsers "users" table Follow-up to #2857 once we confirm we have a backup and all is running smoothly. * Update schema.rb.example

icarito pushed a commit to icarito/plots2 that referenced this pull request Apr 9, 2019

Drop DrupalUsers "users" table (publiclab#4514)

302a79d

* Drop DrupalUsers "users" table Follow-up to publiclab#2857 once we confirm we have a backup and all is running smoothly. * Update schema.rb.example

	def create_drupal_user
	self.bio \|\|= ''
	if drupal_user.nil?
	drupal_user = DrupalUser.new(name: username,
	pass: rand(100_000_000_000_000_000_000),
	mail: email,
	mode: 0,
	sort: 0,
	threshold: 0,
	theme: '',
	signature: '',
	signature_format: 0,
	created: DateTime.now.to_i,
	access: DateTime.now.to_i,
	login: DateTime.now.to_i,
	status: 1,
	timezone: nil,
	language: '',
	picture: '',
	init: '',
	data: nil,
	timezone_id: 0,
	timezone_name: '')
	drupal_user.save!
	self.id = drupal_user.uid
	else
	self.id = DrupalUser.find_by(name: username).uid
	end
	end

Deprecate drupal user model #2857

Deprecate drupal user model #2857

Conversation

Neidley commented Jun 19, 2018

Neidley commented Jun 19, 2018

Neidley commented Jun 21, 2018

jywarren commented Jun 21, 2018

jywarren commented Jun 23, 2018

Neidley commented Jun 23, 2018

jywarren commented Jun 26, 2018 via email

jywarren commented Jul 10, 2018

Neidley commented Jul 11, 2018

jywarren commented Jul 11, 2018

Neidley commented Jul 22, 2018

Neidley commented Jul 22, 2018

grvsachdeva commented Jul 22, 2018

Souravirus commented Jul 22, 2018

Souravirus commented Jul 22, 2018

jywarren commented Jul 23, 2018

Neidley commented Jul 23, 2018

Souravirus commented Jul 23, 2018

Souravirus commented Jul 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren commented Jul 23, 2018 via email

Neidley commented Jul 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neidley commented Jul 24, 2018

Neidley commented Jul 24, 2018

grvsachdeva commented Jan 2, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

jywarren commented Jan 3, 2019

SidharthBansal commented Jan 3, 2019

grvsachdeva left a comment

Choose a reason for hiding this comment

jywarren commented Jan 3, 2019

SidharthBansal commented Jan 3, 2019 via email

Neidley commented Jul 23, 2018 •

edited

Loading