Skip to content
This repository was archived by the owner on Dec 3, 2019. It is now read-only.

Add military_status to user (fixes #373) #375

Merged
merged 13 commits into from
Jul 21, 2018
Merged

Add military_status to user (fixes #373) #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0d4ef35
Add military_status to user
hollomancer Jul 11, 2018
beefd4d
Remove extra empty line
hollomancer Jul 11, 2018
2ad6012
add api blueprint
hollomancer Jul 11, 2018
ab8d9e2
Fix apiary blueprint entry
hollomancer Jul 11, 2018
c0d852e
Add military_status to users_controller and user
hollomancer Jul 12, 2018
046ab05
Update apiary and add constant for MILITARY_STATUSES
hollomancer Jul 16, 2018
b00f188
remove trailing whitespace
hollomancer Jul 16, 2018
0310717
Add failing test for the spaghetti monster
hollomancer Jul 16, 2018
701fb25
Add validation and ActiveAdmin entries
hollomancer Jul 17, 2018
990af71
Fix test
hollomancer Jul 17, 2018
37bdb1b
Update rubocop rules
hollomancer Jul 19, 2018
e7f3704
Remove duplicate input
hollomancer Jul 19, 2018
49c9c09
Update tests
hollomancer Jul 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .rubocop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ Style/StringLiterals:
Metrics/BlockLength:
Exclude:
- 'db/schema.rb'

Metrics/ClassLength:
Exclude:
- 'test/models/user_test.rb'
5 changes: 4 additions & 1 deletion apiary.apib
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,7 @@ API endpoints that Operation Code's Rails backend makes available to its React f
{
errors: "Some error message"
}
## User | Update [/api/v1/users{?education_level,mentor,scholarship_info,employment_status,company_name,company_role,volunteer,branch_of_service,interests}]
## User | Update [/api/v1/users{?education_level,mentor,scholarship_info,employment_status,company_name,company_role,volunteer,military_status,branch_of_service,interests}]

+ Parameters

Expand All @@ -1807,6 +1807,7 @@ API endpoints that Operation Code's Rails backend makes available to its React f
+ company_name (string, optional) - Users name of employer in the form on a string
+ company_role (string, optional) - Users role at company in the form of a string
+ volunteer (boolean, optional) - Is User wanting to volunteer in the form of a boolean
+ military_status (string, optional) - Users military status, either current, veteran, spouse, or blank
+ branch_of_service (string, optional) - Users branch of service in the form of a string
+ interests (string, optional) - User interests in the form of a string.

Expand All @@ -1827,6 +1828,7 @@ API endpoints that Operation Code's Rails backend makes available to its React f
employment_status: "Employed",
company_name: "Comcast",
company_role: "Developer",
military_status: "spouse",
volunteer: "false",
branch_of_service: "Air Force",
interests: "Python, Python, Python, Python"
Expand All @@ -1848,6 +1850,7 @@ API endpoints that Operation Code's Rails backend makes available to its React f
employment_status: "Employed",
company_name: "Comcast",
company_role: "Developer",
military_status: "spouse",
volunteer: "false",
branch_of_service: "Air Force",
interests: "Python, Python, Python, Python"
Expand Down
4 changes: 3 additions & 1 deletion app/admin/user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
:last_name, :timezone, :bio, :verified, :state, :address_1, :address_2, :city,
:username, :volunteer, :branch_of_service, :years_of_service, :pay_grade,
:military_occupational_specialty, :github, :twitter, :linkedin, :employment_status,
:education, :company_role, :company_name, :education_level, :interests
:education, :military_status, :company_role, :company_name, :education_level, :interests

scope :all
scope :mentors
Expand Down Expand Up @@ -74,6 +74,7 @@
column :linkedin
column :employment_status
column :education
column :military_status
column :company_role
column :company_name
column :education_level
Expand Down Expand Up @@ -105,6 +106,7 @@
f.input :volunteer
f.input :branch_of_service
f.input :years_of_service
f.input :military_status, as: :select, collection: User::MILITARY_STATUSES.map { |status| [status, status].compact }.reject(&:empty?), include_blank: true
f.input :pay_grade
f.input :military_occupational_specialty
f.input :github
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/v1/users_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def user_params
:linked_in,
:employment_status,
:education,
:military_status,
:company_role,
:company_name,
:education_level,
Expand Down
7 changes: 7 additions & 0 deletions app/models/user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
class User < ApplicationRecord
LEADER = 'community leader'

# Validation of military_status
CURRENT = 'current'
VETERAN = 'veteran'
SPOUSE = 'spouse'
MILITARY_STATUSES = [CURRENT, VETERAN, SPOUSE, nil]
validates :military_status, inclusion: { in: MILITARY_STATUSES }

# Include default devise modules. Others available are:
# :confirmable, :lockable, :timeoutable and :omniauthable
devise :database_authenticatable, :registerable,
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20180711212702_add_militarystatus_to_user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddMilitarystatusToUser < ActiveRecord::Migration[5.0]
def change
add_column :users, :military_status, :string
Copy link
Collaborator

Choose a reason for hiding this comment

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

As this is a string, is the plan to accept any string, or a specific list of strings for consistency? Meaning a specified set of four statuses, for example. I highly recommend the latter.

This way, we can enforce data integrity, hang logic on the responses, do data analysis, filtering and searching, add rules for certain statuses, etc., etc.

If the answer is yes, then we'll need to do a few extra additions in this PR before shipping it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree. Let's do it the right way.

From the frontend, current, veteran, spouse are the values provided.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd just need to add the check in /app/models/user.rb, is that correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hollomancer - To establish the specific set of validated statuses, you'll want to:

  1. Create a constant for each status here.
  2. Create a constant to represent an array of all of the statuses. You'll need to include one for nil, to account for our current users, and future users, that will not have a status. For example:
CURRENT = 'current'
VETERAN = 'veteran'
SPOUSE = 'spouse'
MILITARY_STATUSES = [CURRENT, VETERAN, SPOUSE, nil]
  1. In models/user.rb, Add an inclusion validation for the new :military_status attribute, for the MILITARY_STATUSES. Here is how to do that.
  2. In admin/user.rb, add the set of selections to ActiveAdmin dashboard form portion. For example:
f.input :military_status, as: :select, collection: User::MILITARY_STATUSES.map { |status| [status, status].compact }.reject(&:empty?), include_blank: true
  1. Add test coverage to user_test.rb testing the new inclusion validation
  2. In apiary.apib, in the military_status definition, change that to something like:
Users military status, either: current, veteran, spouse, or blank

end
end
Loading