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

Set MYSQL version to 5 (5.7 current). Added past events in db:seed. #76

Closed
wants to merge 24 commits into from

Conversation

sunilkr
Copy link
Collaborator

@sunilkr sunilkr commented Apr 4, 2021

#68
Changed MYSQL container version to 5 in (5.7 current) only in non-production environments. Requires a 'docker-compose build and mysql_upgrade to DB to work correctly.

#70
Added past event data in seed.rb, but to get everything working, I have to change event.slugify! from after_create to before_create and comment save().

sunilkr and others added 21 commits July 7, 2019 09:24
@sunilkr
Copy link
Collaborator Author

sunilkr commented Apr 4, 2021

@anantshri @abhisek please review.

@sunilkr
Copy link
Collaborator Author

sunilkr commented May 3, 2021

@abhisek @anantshri please review.

@sunilkr sunilkr changed the title Set MYSQL version to 5 (5.7 current) Set MYSQL version to 5 (5.7 current). Added past events in db:seed. May 3, 2021
@anantshri
Copy link
Member

you mentioned 5.7 but added 5.5 rest all looks good to me.

@sunilkr
Copy link
Collaborator Author

sunilkr commented May 24, 2021

you mentioned 5.7 but added 5.5 rest all looks good to me.

Actually I kept only mysql5 (which currently is 5.7) instead of mysql5.5. ".5" is removed from yml files.

@anantshri
Copy link
Member

you mentioned 5.7 but added 5.5 rest all looks good to me.

Actually I kept only mysql5 (which currently is 5.7) instead of mysql5.5. ".5" is removed from yml files.

Ah my bad, not sure what i was thinking i read the commits inverse.

LGTM 👍

@@ -1,7 +1,7 @@
version: '2'
services:
db:
image: mysql:5.5
image: mysql:5
Copy link
Member

Choose a reason for hiding this comment

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

Explicitly set the minor version for MySQL

@@ -147,7 +148,7 @@ def to_param
# ActiveRecord :on_create
def slugify!
self.slug = "#{self.chapter.name} #{self.name.parameterize} #{self.id}".parameterize
self.save()
#self.save()
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Collaborator Author

@sunilkr sunilkr Jul 2, 2021

Choose a reason for hiding this comment

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

I had to explicitly call event.save and wanted to have slug before save.

e.save(:validate => false)

@sunilkr
Copy link
Collaborator Author

sunilkr commented Aug 1, 2021

@abhisek PR Updated.

@@ -147,7 +148,7 @@ def to_param
# ActiveRecord :on_create
def slugify!
self.slug = "#{self.chapter.name} #{self.name.parameterize} #{self.id}".parameterize
Copy link
Member

Choose a reason for hiding this comment

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

self.id is available only after DB persistence. It will not be available in before_create

@@ -36,7 +36,8 @@ class Event < ActiveRecord::Base
has_many :event_registrations, :dependent => :destroy

before_create :set_automatic_values! # Defaults
after_create :slugify!
#after_create :slugify!
before_create :slugify!
Copy link
Member

Choose a reason for hiding this comment

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

Breaks self.id reference in slugify

@sunilkr
Copy link
Collaborator Author

sunilkr commented Apr 10, 2022

Abandoning this. Will separate this into 2 PRs.

@sunilkr sunilkr closed this Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants