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.

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

Merge profiles into users WIP #109

Merged
merged 40 commits into from
May 13, 2015
Merged

Merge profiles into users WIP #109

merged 40 commits into from
May 13, 2015

Conversation

jforberg
Copy link
Contributor

Profiles will be merged into users.

The migration is done, now we have to fix up all the references to Profile that still remain.

A good command to use for checking what is left to do:

$ egrep -Ri 'profiles?' app lib config

@@ -0,0 +1,96 @@
class MergeProfileIntoUser < ActiveRecord::Migration
def change

Choose a reason for hiding this comment

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

Assignment Branch Condition size for change is too high. [36/15]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wtf

@henrikssn
Copy link
Contributor

You should split this into 3 migrations:

  1. Create new columns (not rename)
  2. Big transaction where you copy over all data
  3. Delete old columns

elsif((!@dokument.public) && (current_user) && (@dokument.pdf_file_name) || (@dokument.public))
send_file(@dokument.pdf.path, filename:@dokument.pdf_file_name, type: "application/pdf",disposition: 'inline',x_sendfile: true)
elsif ((!@dokument.public) && (current_user) && (@dokument.pdf_file_name) || (@dokument.public))
send_file(@dokument.pdf.path, filename: @dokument.pdf_file_name, type: "application/pdf", disposition: 'inline', x_sendfile: true)

Choose a reason for hiding this comment

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

Line is too long. [136/100]
Prefer single-quoted strings when you don't need string interpolation or special symbols.

@davidwessman
Copy link
Member

I realized it might be bad to make all those changes in one commit.
I'll save my state as a local branch incase we want to roll-back and split it up.

ef42d70

@jforberg jforberg changed the title Merge profiles into users Merge profiles into users WIP Mar 25, 2015
@davidwessman davidwessman mentioned this pull request Mar 29, 2015
5 tasks
@davidwessman
Copy link
Member

I will look at this some more now, because refactoring other code that depends so much on profiles is not that nice before this is done.

return
end
send_file(@document.pdf.path, filename: @document.pdf_file_name,
type: 'application/pdf', disposition: 'inline', x_sendfile: true)

Choose a reason for hiding this comment

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

Align the elements of a hash literal if they span more than one line.

@davidwessman
Copy link
Member

@fsek/admins

I think I'm done. Implemented some new feature tests, fixed a lot of views.
There is still a lot of work with changing models according to #178.

Now it will be easier to fix #52 and other PR's with users.

Please tests this out a bit locally, and on dev if you have time.

@henrikssn
Copy link
Contributor

Pushed to dev

insert into post_users (post_id, user_id)
select post_id, profile_id as user_id from posts_profiles
eof

Choose a reason for hiding this comment

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

Extra empty line detected at method body end.

@henrikssn
Copy link
Contributor

@fsek/admins Jag har fixat de fel jag hittat. Prova gärna denna PR på dev.fsektionen.se och säg till om ni hittar några fel.

Annars tycker jag vi mergar denna :)

config.action_mailer.raise_delivery_errors = true
config.action_mailer.default charset: 'utf-8'
config.action_mailer.raise_delivery_errors = false
config.action_mailer.default :charset => "utf-8"

Choose a reason for hiding this comment

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

Use the new Ruby 1.9 hash syntax.
Prefer single-quoted strings when you don't need string interpolation or special symbols.

cafe_work: attributes_for(:assignee, :test))
cwork_profile.reload
patch(:update_worker, id: cwork_worker.to_param,
cafe_work: attributes_for(:assignee, :test))

Choose a reason for hiding this comment

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

Align the elements of a hash literal if they span more than one line.

davidwessman added a commit that referenced this pull request May 12, 2015
@henrikssn
Copy link
Contributor

Lets merge this! I mean, it can't break anything, right? 👼

@henrikssn
Copy link
Contributor

No but seriously, we have tested the migration on dev and it did work fine there. I will take a db backup before I merge.

henrikssn added a commit that referenced this pull request May 13, 2015
Merge profiles into users WIP
@henrikssn henrikssn merged commit d9a72a2 into master May 13, 2015
@ghost
Copy link

ghost commented May 13, 2015

This PR was deployed to Production. Reference: d9a72a2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants