Fix SSO again

[bpurinton]

Resolves https://github.com/firstdraft/learn/issues/1971

Resolves #8

Problem: Heroku deployment is not gonna work with the current Ruby version here. May try Render.

Attempted Render deployment dashboard: https://dashboard.render.com/project/prj-d1claaqdbo4c73cvmga0/environment/evm-d1claaqdbo4c73cvmgag

Need to update to Ruby v3.1, which also probably means updating a lot of other things.. might need to just start from scratch on this app..

---

Important

Update Ruby version, dependencies, and configurations for Render deployment, and fix SSO handling in User model.

• Environment:
  • Update Ruby version to 3.1.3 in .ruby-version and Gemfile.
  • Add render.yaml for Render deployment configuration.
  • Add bin/render-build.sh for build commands on Render.
  • Comment out bootsnap in Gemfile and config/boot.rb for compatibility.
  • Add logger gem and require in config/boot.rb and config/initializers/fix_logger.rb.
• Dependencies:
  • Update activeadmin to ~> 2.9.
  • Update rails to ~> 6.1.0.
  • Update omniauth-github to ~> 1.4.
  • Update listen to ~> 3.8.
  • Add psych gem with version < 4.
• Models:
  • Modify User.from_omniauth in user.rb to find users by provider and uid instead of email.
  • Ensure email is stored in lowercase in User.from_omniauth.
• Views:
  • Minor HTML structure change in _navbar.html.erb by adding a closing div.

^{This description was created by for dbf37d9. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Skipped PR review on 922bcce because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to f9a2590 in 1 minute and 27 seconds. Click for details.

• Reviewed 40 lines of code in 2 files
• Skipped 1 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. app/views/shared/_navbar.html.erb:32

• Draft comment:
  Fixed unclosed container div. The added properly closes the container inside the navbar.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_9EuIzCI3MQN9lM5W

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed 361e9fa in 1 minute and 28 seconds. Click for details.

• Reviewed 43 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. bin/render-build.sh:6

• Draft comment:
  Ensure that running 'assets:clean' after 'assets:precompile' is intentional and won’t remove required assets.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment starts with "Ensure that..." which is explicitly called out in our rules as likely not useful. It's asking the author to verify their intention rather than pointing out a clear issue. assets:clean is a standard Rails command that removes old compiled assets while keeping the current ones - it's commonly used after precompile. Maybe there could be a real issue with assets:clean removing needed files in some edge cases? Maybe this is a legitimate concern about build script correctness? No, assets:clean is specifically designed to work after precompile - it only removes old compiled assets while preserving the newly compiled ones. This is a standard Rails pattern. The comment should be deleted as it violates our rules by asking for verification rather than pointing out a clear issue, and it questions a standard Rails asset pipeline pattern.

Workflow ID: wflow_QX4fkX92rxhOrOIz

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Script is missing executable permissions (mode 100644). It should be executable (e.g. mode 100755).

[ellipsis-dev]

DATABASE_URL's fromDatabase name 'envoy' does not match the defined database 'association-accessors'.

Suggested change

	name: envoy
	name: association-accessors

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 75afb99 in 1 minute and 11 seconds. Click for details.

• Reviewed 23 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. render.yaml:3

• Draft comment:
  Naming inconsistency: the database block uses 'association-accessors' for its name, but 'databaseName' and 'user' use underscores. Confirm if this is intentional per Render’s requirements.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. render.yaml:21

• Draft comment:
  DATABASE_URL now references 'association-accessors' instead of 'envoy'. Ensure this correctly links to the intended database.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_VjKqvufl693b7rDT

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed df191ba in 1 minute and 19 seconds. Click for details.

• Reviewed 20 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 5 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. render.yaml:6

• Draft comment:
  Verify 'basic' database plan is intentional over 'starter'.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is exactly the type of comment we should remove. It's asking the author to verify their intention, which violates our rules. The change from starter to basic was clearly deliberate as it was done consistently in both places. Infrastructure changes like this are typically intentional business decisions. Maybe the change could have serious cost implications that should be double-checked? Maybe this is a security concern? Plan changes are business decisions that would have been approved before the PR. The author clearly meant to make this change as they did it systematically. Delete this comment. It violates our rule about not asking authors to verify their intentions, and the change appears deliberate and systematic.

2. render.yaml:12

• Draft comment:
  Confirm 'basic' service plan meets performance requirements compared to 'starter'.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm performance requirements, which is against the rules. It doesn't provide a specific code suggestion or point out a specific issue in the code.

3. render.yaml:16

• Draft comment:
  PreDeployCommand for migrations is added; ensure it runs successfully in production.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. render.yaml:17

• Draft comment:
  Ensure that the Puma configuration in config/puma.rb exists and is properly set up.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. render.yaml:18

• Draft comment:
  Check that envVars (DATABASE_URL and WEB_CONCURRENCY) are correctly configured for your environment.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_WekqAGapeYteQBuk

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 626b031 in 51 seconds. Click for details.

• Reviewed 20 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 6 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. render.yaml:6

• Draft comment:
  Verify that 'basic-256mb' is a valid plan meeting memory needs.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. render.yaml:12

• Draft comment:
  Ensure 'starter' service plan is appropriate for production loads.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. render.yaml:16

• Draft comment:
  Confirm preDeployCommand (db:migrate) runs safely and idempotently.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. render.yaml:17

• Draft comment:
  Verify the startCommand for puma uses the correct configuration.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

5. render.yaml:19

• Draft comment:
  Check DATABASE_URL mapping; ensure it retrieves the correct connection string.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

6. render.yaml:23

• Draft comment:
  Confirm that WEB_CONCURRENCY=2 is optimal for the app workload.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_TxcOtE69oJlg4brv

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed d9a1e74 in 1 minute and 39 seconds. Click for details.

• Reviewed 63 lines of code in 4 files
• Skipped 1 files when reviewing.
• Skipped posting 5 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. .ruby-version:1

• Draft comment:
  Upgraded Ruby to 3.0.3; confirm all dependencies are Ruby 3 compatible.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

2. Gemfile:4

• Draft comment:
  Rails has been updated to ~>6.1.0 and ActiveAdmin to ~>2.10; verify no breaking changes affect functionality (including SSO).
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

3. Gemfile:8

• Draft comment:
  Bootsnap has been commented out, which may affect boot performance. Consider updating it for Ruby 3 compatibility instead of disabling.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

4. config/boot.rb:4

• Draft comment:
  Requiring logger early is a workaround for Ruby 3; if bootsnap is re-enabled later, remove this workaround to maintain boot optimization.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

5. config/initializers/fix_logger.rb:1

• Draft comment:
  This initializer requires 'logger' but lacks a newline at EOF. Ensure proper file formatting.
• Reason this comment was not posted:
  Confidence changes required: 10% <= threshold 50% None

Workflow ID: wflow_UhZlWSISghMJvMjS

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Including 'logger' as a gem is unusual since Ruby provides it in the stdlib; confirm if a custom version is needed.

Suggested change

gem 'logger'

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 465ab17 in 1 minute and 26 seconds. Click for details.

• Reviewed 50 lines of code in 2 files
• Skipped 1 files when reviewing.
• Skipped posting 8 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. .ruby-version:1

• Draft comment:
  Ruby version bumped to 3.1.3. Ensure deployment environments support it.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is related to a dependency change, specifically the Ruby version. The comment suggests ensuring deployment environments support the new version, which is a form of asking the author to double-check something. This violates the rules against asking for confirmation or ensuring compatibility.

2. Gemfile:4

• Draft comment:
  Updated ruby version to 3.1.3. Check that all gems are compatible.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to check for compatibility of gems with the updated Ruby version. This falls under the rule of not asking the author to ensure compatibility or double-check things, which is not allowed.

3. Gemfile:6

• Draft comment:
  Bump activeadmin to ~> 2.13. Verify there are no breaking changes.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is related to a dependency change, specifically bumping the version of activeadmin. The comment asks to verify that there are no breaking changes, which is against the rules as it asks the PR author to ensure something. Therefore, this comment should be removed.

4. Gemfile:10

• Draft comment:
  Lock psych gem (< 4) to avoid compatibility issues. Consider adding a comment for context.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

5. Gemfile:31

• Draft comment:
  Updated rubocop-rails to ~> 2.17. Ensure CI and lint rules are adjusted if needed.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is related to a dependency change, specifically updating the rubocop-rails version. The comment suggests ensuring CI and lint rules are adjusted if needed, which falls under asking the PR author to ensure something is tested or verified. This violates the rules.

6. Gemfile:33

• Draft comment:
  Set shoulda-matchers to ~> 5.0. Confirm compatibility with Rails 6.1 and test suite.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to confirm compatibility and test the suite, which violates the rules against asking for confirmation or testing. It also involves dependency changes, which should not be commented on unless the library version is recognized and relevant.

7. Gemfile:38

• Draft comment:
  Pinned omniauth-github to ~> 1.4. Validate integration with SSO flows.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is about a dependency change, specifically pinning a version of omniauth-github. It suggests validating integration with SSO flows, which is a request to ensure the change is tested. This violates the rule against asking the PR author to ensure the change is tested.

8. Gemfile:42

• Draft comment:
  Updated factory_bot_rails to ~> 6.2. Ensure tests run correctly with new version.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is related to a dependency update and asks the author to ensure tests run correctly, which violates the rules against commenting on dependencies and asking for confirmation or testing.

Workflow ID: wflow_hDCuLAFqleUZu1lg

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed dbf37d9 in 1 minute and 31 seconds. Click for details.

• Reviewed 34 lines of code in 1 files
• Skipped 1 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. Gemfile:6

• Draft comment:
  Ensure the downgrade of activeadmin from '> 2.13' to '> 2.9' is intentional; verify that no required features or fixes are lost.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to ensure that the downgrade of a dependency is intentional and to verify that no features or fixes are lost. This falls under the rule of not asking the author to confirm their intention or to ensure behavior is intended. Therefore, this comment should be removed.

2. Gemfile:31

• Draft comment:
  Confirm that pinning rubocop-rails to '2.5.2' (downgraded from '~> 2.17') is deliberate; this might miss newer lint rules or fixes.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

3. Gemfile:33

• Draft comment:
  Removing the version constraint for shoulda-matchers can unintentionally upgrade to a breaking version. Consider pinning a stable version.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% According to the rules, we should NOT comment on dependency changes or library versions. The rules explicitly state to ignore dependency-related changes. While the suggestion about version pinning might be valid, it's outside the scope of what we should comment on. The comment does raise a valid software engineering concern about dependency management and potential breaking changes. However, the rules are clear that dependency management and version changes are explicitly out of scope for these reviews. The comment should be deleted as it relates to dependency management, which is explicitly excluded from the review scope.

4. Gemfile:42

• Draft comment:
  Removing the version constraint for factory_bot_rails may lead to unintended updates; consider pinning to '~> 6.2' or a specific version.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a dependency version change. The rules explicitly state not to comment on dependency changes or library versions. While version pinning can be important for stability, we should trust the author's judgment on dependency management. Multiple other gems in the file also don't have version constraints. Version constraints are important for maintaining stable builds and preventing unexpected breaking changes. The comment has valid concerns about potential future issues. However, the rules clearly state to NOT comment on dependency changes or library versions. We must trust the author's judgment on dependency management. Delete the comment as it violates the rule about not commenting on dependency changes and library versions.

Workflow ID: wflow_Goz8ztVPo22P24Ry

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}