Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
a9c7daf
LG-6625: Hide IPP troubleshooting option for failures (#6597)
aduth Jul 26, 2022
7a0e814
remove proofing post office and city from log (#6631)
Jul 26, 2022
c8a185b
LG-6343: Implement email template for "Ready to verify" (#6585)
aduth Jul 27, 2022
b25790b
Send "Ready to verify" email when IPP enrollment is created (#6630)
aduth Jul 27, 2022
6650d8e
LG-6992: resolve secure header issues (#6625)
mdiarra3 Jul 27, 2022
cd464e7
LG-6897/LG-6868/LG-6437: Set pending profile state for IPP (#6634)
aduth Jul 27, 2022
fee0abe
LG-6089 Add location page to in-person flow (#6624)
Jul 27, 2022
17eddab
LG_6949 Add AttemptsAPI Logout: SessionsController (#6636)
ThatSpaceGuy Jul 27, 2022
ea46203
log success, add enrollment code to events (#6640)
Jul 27, 2022
9a06c7e
Initialize IPP pii_from_user with user UUID (#6641)
aduth Jul 27, 2022
c9c25bf
Clean up CONTRIBUTING.md, update docs/backend.md (#6639)
zachmargolis Jul 27, 2022
b665ce8
LG-6872: Create IPP enrollments in FSM v1 path (#6635)
Jul 27, 2022
1c84c38
LG-7004-configs-threatmetrix (#6637)
gsa-manish Jul 28, 2022
095b0c4
Use selected location details from session (#6645)
aduth Jul 28, 2022
3cd4e57
LG-7605 Small changes to in-person enrollment email copy (#6646)
Jul 28, 2022
c6ccbf1
Upgrade to Rails 7 (#5762)
Jul 28, 2022
54443ba
LG-7035: Clear in-person proofing session when starting over (#6650)
aduth Jul 28, 2022
cf12f73
LG-7063: Avoid sending account verified email for in-person proofing …
aduth Jul 28, 2022
7a449f6
Add feature flags for Device Profiling (#6644)
theabrad Jul 28, 2022
0c07eb4
Fix comments on ErrorsAddLinter (#6653)
zachmargolis Jul 28, 2022
8850ab8
Use correct Spanish label for "Retail hours" (#6656)
aduth Jul 28, 2022
93e8c8f
add precision values to datetime columns in schema (#6657)
Jul 29, 2022
8b0f4c2
LG-6385: Update translated text from Google translate to Gengo round …
lizzieamanning Jul 29, 2022
515d605
Show "Cancel" links for in-person proofing steps (#6662)
aduth Aug 1, 2022
2a23ace
[LG-6797] VA Request and Response for User PII (#6608)
gangelo Aug 1, 2022
6285253
LG-6089: Pass location selection between pages (#6666)
Aug 1, 2022
3859166
Add a form to validate PII returned from VA inherited proofing (#6642)
gangelo Aug 1, 2022
389b186
Add Pinpoint Check to GitLab CI (#6614)
Aug 1, 2022
0f87558
Enable Numeric OTP by default (#6670)
Aug 1, 2022
1986d42
Remove puts in test (#6672)
Aug 1, 2022
32ec055
LG-6436: Add hybrid document capture opt-in for in-person proofing (#…
aduth Aug 2, 2022
d8c8ed8
LG-6785: Upgrade Acuant SDK to 11.7.0 (#6649)
eric-gade Aug 2, 2022
fb8cae3
LG-6998: add additional logging for mfa setup on account creation flo…
mdiarra3 Aug 2, 2022
8205a50
LG-6341: Create template for success email and send it when status is…
Aug 2, 2022
3336cc2
LG-6845: Fix "Try again" button and SSN "< Back" link/button for IPP …
NavaTim Aug 2, 2022
f781164
Add security event for password resets (#6668)
orenyk Aug 2, 2022
6af22db
hide broken help links on in-person proofing pages (#6679)
Aug 2, 2022
83d0b43
[LG-6902] Validate data from VA IP API which amounts to presence: tru…
gangelo Aug 3, 2022
abd5e7b
Lg 6899 decrypting va data (#6667)
holytoastr Aug 3, 2022
9b192cd
Fix 500 errors when handling unsafe redirects (#6682)
Aug 3, 2022
5e77236
changelog: Bug Fixes, Authentication, AAl3 Users cannot skip MFA sele…
mdiarra3 Aug 3, 2022
ea46a53
LG-6668 safeguard failed message (#6647)
peggles2 Aug 3, 2022
a833294
LG-7132: Make the date parsing in TrueID Response more robust. (#6683)
solipet Aug 3, 2022
94944f8
Merge remote-tracking branch 'origin/stages/prod' into stages/rc-2022…
solipet Aug 3, 2022
c81ec2c
Instrument HTTP requests for In-Person-Proofing (#6685)
Aug 3, 2022
3c9b207
Fix broken specs (#6689)
Aug 3, 2022
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
13 changes: 12 additions & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ install:

specs:
stage: test
parallel: 7
parallel: 11
cache:
- <<: *ruby_cache
- <<: *yarn_cache
Expand Down Expand Up @@ -185,6 +185,17 @@ js_tests:
- *yarn_install
- yarn test


pinpoint-check:
stage: test
cache:
- <<: *ruby_cache
- <<: *yarn_cache
script:
- *bundle_install
- *yarn_install
- make lint_country_dialing_codes

coverage:
stage: after_test
cache:
Expand Down
134 changes: 0 additions & 134 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,123 +55,6 @@ changelog categories.

[changelog check script]: https://github.com/18F/identity-idp/blob/main/scripts/changelog_check.rb

### Style, Readability, and OO
- Rubocop or Reek offenses are not disabled unless they are false positives.
If you're not sure, please ask a teammate.

- Related methods in the same class are in descending order of abstraction.
This is best explained through this video: https://youtu.be/0rsilnpU1DU?t=554

- Compound conditionals are replaced with more readable methods that describe
the business rule. For example, a conditional like
`user_session[:personal_key].nil? && current_user.personal_key.present?` could
be extracted into a method called
`current_user_has_already_confirmed_their_personal_key?`.
Another example is explained in this video: https://youtu.be/0rsilnpU1DU?t=40s

- Service Objects should usually only have one public method, usually named
`call`. This mostly applies to classes that perform a specific task, unlike
Presenters, View Objects, and Value Objects, for example. Read
[7 Patterns to Refactor Fat ActiveRecord Models] for a good overview of the
different types of classes used in Rails.

References:
- https://medium.com/selleo/essential-rubyonrails-patterns-part-1-service-objects-1af9f9573ca1
- https://multithreaded.stitchfix.com/blog/2015/06/02/anatomy-of-service-objects-in-rails/
- https://hackernoon.com/the-3-tenets-of-service-objects-c936b891b3c2
- http://katafrakt.me//2018/07/04/writing-service-objects/
- https://pawelurbanek.com/2018/02/12/ruby-on-rails-service-objects-and-testing-in-isolation/

[7 Patterns to Refactor Fat ActiveRecord Models]: https://codeclimate.com/blog/7-ways-to-decompose-fat-activerecord-models/

### RESTful controllers

* Only use CRUD methods in controllers.

* Prefer adding a new controller with one of the CRUD methods over creating a
custom method in an existing controller. For example, if your app allows a
user to update their email and their password on two different pages, instead of
using a single controller with methods called `update_email` and
`update_password`, create two controllers and name the methods `update`, i.e.
`EmailsController#update` and `PasswordsController#update`. See
http://jeromedalbert.com/how-dhh-organizes-his-rails-controllers/ for more about
this design pattern.

### Lean controllers
* Keep as much business logic as possible out of controllers.

* Use specialized classes to handle the operations
* These will be Form Objects for the most part, since
most of what the app does is process user input via a form submission, or
clicking a link in email that contains a token.

* Form Object rules:
- Should have a single public method called `submit` that returns a [FormResponse] object.
- Should use ActiveModel validations to validate the user input.
- Should be placed in `app/forms`.

* Examples of Form/Service Objects:
- [EmailConfirmationTokenValidator]
- [PasswordForm]

* The basic outline of how a controller interacts with this class is:
```ruby
result = Class.new(user).submit(params) # this returns a FormResponse object
# all the necessary analytics attributes are captured inside the Form Object
analytics.track_event('Some Event Name', result.to_h)

if result.success?
handle_success
else
handle_failure
end
```

* Only make one call to `analytics.track_event` after submitting the form, as
opposed to one call when handling success and another when handling failure. The
Form Object, when used properly, will return a FormResponse object that already
tells us whether the action was successful or not.

### Importance of the controller design for analytics

This design pattern was the result of many iterations, and agreed upon by all
team members in early 2017. It keeps controllers clean and predictable. Having a
controller interact with a Form Object or some other specialized class is not a
new concept. Rails developers have been using them since at least 2012. What
might seem new is the `FormResponse` object. **The most important reason
controllers expect an object that responds to `success?` and `to_h` is to define
an analytics API, or a contract, if you will, between the analytics logs and the
folks who query them.**

For example, if someone wants to look up all events that have failed, they would
run this query in Kibana: `properties.event_properties.success:false`. Now let's
say a developer introduces a new controller that doesn't adhere to our established
convention, and captures analytics in their own way, without adding `success`
and `errors` keys, which are expected to be included in all analytics events.
This means that any failures for this controller won't show up when running the
query above, and the person running the query might not realize data is missing.

Deviating from the convention also causes confusion. The next developer to join
the team will not be sure which pattern to use, and might end up picking the
wrong pattern. As Sandi Metz says:

> For better or for worse, the patterns you establish today will be replicated
forever. When the code lies you must be alert to programmers believing and then
propagating that lie.

### Secure controllers
Rails by default is currently vulnerable to [cache poisoning attacks]
through modification of the `X-Forwarded-For` and `Host` headers. In
order to protect against the latter, there are two pieces that must be
in place. The first one is already taken care of by defining
`default_url_options` in `ApplicationController` with a `host` value
that we control.

The other one is up to you when adding or modifying redirects:

- Always use `_url` helpers (as opposed to `_path`) when calling
`redirect_to` in a controller.

### Additional notes on pull requests and code reviews

Please follow our [Code Review][review] guidelines.
Expand All @@ -181,27 +64,10 @@ reading.
[review]: https://engineering.18f.gov/code-review/
[thoughts]: http://glen.nu/ramblings/oncodereview.php

- Prioritize code reviews for the current sprint above your other work
- Review pull requests for the current sprint within 24 hours of being opened
- Keep pull requests as small as possible, and focused on a single topic
- Once a pull request is good to go, the person who opened it squashes related
commits together, merges it, then deletes the branch.

### Recommended reading, viewing, and courses

- [Practical Object-Oriented Design in Ruby](http://www.poodr.com/)
- [99 Bottles of OOP](https://sandimetz.dpdcart.com/)
- [Sandi Metz blog](https://www.sandimetz.com/blog/)
- [Sandi Metz talks](https://www.youtube.com/playlist?list=PLFQBiiaZoyrcTBYAGAUjvEUI6TUrp110W)
- [Learn Clean Code](https://thoughtbot.com/upcase/clean-code)
- [Ruby Science](https://gumroad.com/l/ruby-science)
- [Ruby Tapas](https://www.rubytapas.com/)
- [Master the Object-Oriented Mindset in Ruby and Rails](https://avdi.codes/moom/)
- [Refactoring Rails](https://www.refactoringrails.io/)
- [Growing Rails Applications in Practice](https://pragprog.com/book/d-kegrap/growing-rails-applications-in-practice)
- [The 30-Day Code Quality Challenge](https://www.codequalitychallenge.com/)
- [SourceMaking](https://sourcemaking.com/)

## Public domain

This project is in the public domain within the United States, and
Expand Down
11 changes: 6 additions & 5 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}.git" }

ruby "~> #{File.read('.ruby-version').strip}"

gem 'rails', '~> 6.1.6.1'
gem 'rails', '~> 7.0.0'

gem 'ahoy_matey', '~> 3.0'
gem 'aws-sdk-kms', '~> 1.4'
Expand All @@ -26,7 +26,7 @@ gem 'foundation_emails'
gem 'good_job', '~> 2.99.0'
gem 'hashie', '~> 4.1'
gem 'http_accept_language'
gem 'identity-hostdata', github: '18F/identity-hostdata', tag: 'v3.4.0'
gem 'identity-hostdata', github: '18F/identity-hostdata', tag: 'v3.4.1'
gem 'identity-logging', github: '18F/identity-logging', tag: 'v0.1.0'
gem 'identity_validations', github: '18F/identity-validations', tag: 'v0.7.2'
gem 'jsbundling-rails', '~> 1.0.0'
Expand Down Expand Up @@ -60,6 +60,7 @@ gem 'safe_target_blank', '>= 1.0.2'
gem 'saml_idp', github: '18F/saml_idp', tag: '0.17.0-18f'
gem 'scrypt'
gem 'simple_form', '>= 5.0.2'
gem 'sprockets-rails'
gem 'stringex', require: false
gem 'strong_migrations', '>= 0.4.2'
gem 'subprocess', require: false
Expand Down Expand Up @@ -89,7 +90,7 @@ end
group :development, :test do
gem 'aws-sdk-cloudwatchlogs', require: false
gem 'brakeman', require: false
gem 'bullet', '>= 6.0.2'
gem 'bullet', '~> 7.0'
gem 'capybara-webmock', git: 'https://github.com/hashrocket/capybara-webmock.git', ref: '63d790a0'
gem 'data_uri', require: false
gem 'erb_lint', '~> 0.1.0', require: false
Expand All @@ -103,7 +104,7 @@ group :development, :test do
gem 'pry-rails'
gem 'psych'
gem 'puma'
gem 'rspec-rails', '~> 4.0'
gem 'rspec-rails', '6.0.0.rc1'
gem 'rubocop', '~> 1.29.1', require: false
gem 'rubocop-performance', '~> 1.12.0', require: false
gem 'rubocop-rails', '>= 2.5.2', require: false
Expand All @@ -117,7 +118,7 @@ group :test do
gem 'simplecov-cobertura'
gem 'simplecov_json_formatter'
gem 'email_spec'
gem 'factory_bot_rails', '>= 5.2.0'
gem 'factory_bot_rails', '>= 6.2.0'
gem 'faker'
gem 'rack_session_access', '>= 0.2.0'
gem 'rack-test', '>= 1.1.0'
Expand Down
Loading