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

Updated Gem to support Rails 6/7 #48

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

a-figueiredo-campos
Copy link

@a-figueiredo-campos a-figueiredo-campos commented Feb 1, 2023

  • Added Rspec
    - Updated tests for spec
    - Updated factories for factory_bot
  • Added Appraisal
  • Refactored gem for Rails 6/7

Added Rspec and Appraisals with minor tweaks

Added Rspec and Appraisals

Updated gitignore

Updated Appraisals gemfiles
@a-figueiredo-campos a-figueiredo-campos changed the title Updated Gem to rails 7 Updated Gem to support Rails 6/7 Feb 3, 2023
@a-figueiredo-campos a-figueiredo-campos marked this pull request as ready for review February 3, 2023 15:29
scope :looses, -> { where(winner: false) }
scope :scores, -> { order('score DESC') }
scope :for_survey, ->(survey) { where(survey_id: survey.id) }
scope :exclude_survey, ->(survey) { where("NOT survey_id = #{survey.id}") }
Copy link
Member

Choose a reason for hiding this comment

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

where.not(survey_id: survey.id)

end

def collect_scores
answers.map(&:value).reduce(:+) || 0
Copy link
Member

Choose a reason for hiding this comment

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

We are changing the behavior here by not setting the self.score field like we used to

@@ -23,26 +25,26 @@ def create_resolution
private

def generate_active_admin_resolution
copy_file "active_admin.rb", "app/admin/survey.rb"
copy_file 'active_admin.rb', 'app/admin/survey.rb', force: true
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add the force flag?

Copy link
Author

Choose a reason for hiding this comment

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

Forgot to remove, it was from previous tests

lib/survey.rb Outdated
ActiveRecord::Base.include Survey::ActiveRecord
Copy link
Member

Choose a reason for hiding this comment

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

Rails 5 added the ApplicationRecord. If the file exists, we should use it instead of ActiveRecord::Base

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm okok, i will change it now 👍🏼

VERSION = '0.1'
Copy link
Member

Choose a reason for hiding this comment

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

let's bump this

Copy link
Author

Choose a reason for hiding this comment

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

Bump version to 0.2?

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.

2 participants