forked from DMPRoadmap/roadmap
-
Notifications
You must be signed in to change notification settings - Fork 0
Development #4
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
Open
xsrust
wants to merge
2,399
commits into
xsrust:development
Choose a base branch
from
DMPRoadmap:development
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Development #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tinymce v6
Translation sync from Translation.io
…uby 3 and Tinymce 6
v4.1.0 - release candidate
removed prerequisites section since it's already on the installation wiki
Merge latest from main into development
…a, Sans-Serif' to 'Roboto, Arial, Sans-Serif'.
Remove Auto-Generated TinyMCE Skins and Add `public/tinymce/skins/` to `.gitignore`
Contributed by @gjacob24 of DCC DMPonline. The changes: - Replaced Selection_tag classes incorrectly set with 'form-control' to 'form-select'. Co-authored-by: gjacob24 <[email protected]>
Contributed by @gjacob24 of DCC DMPonline. Changes: - Adjusts the layout and displays the 'optional plan components' and 'select phase to download' side by side for multi-phase plans. Co-authored-by: gjacob24 <[email protected]>
- This changes expand upon commit 87ddbb0
- This change improves the styling/positioning between the "Optional plan components" and "Select phase to download" headings.
- Added "form-control" class to "Optional plan components" heading to match "Select phase to download" stylings
- Replaced `<legend>` element with a `label_tag`+ `form-label` class to again improve consistency between the two headings.
- Note, the added `label_tag` doesn't associate with any specific form control, so the first paramater is left empty.
…of-style-of-select-tags-and-plan-download-layout Additional `form-control` to `form-select` changes & styling changes to "Download settings" subheadings
…ect-tags-and-plan-download-layout
- Commit 72c2519 improved the alignment between the 'Optional plan components' and 'Select phase to download' headings. However, the change from <legend> to an unreferenced label_tag() hurt the accessibility. - This change puts back the <legend> tag and uses class: 'col-form-label' for both of the aforementioned headings to maintain stylistic consistency between the two elements. - `.col-form-label` overrides <legend> styling to maintain consistent alignment with <label> elements. - Accessibility snapshot of <fieldset> before this fix: Name: "" aria-labelledby: Not specified aria-label: Not specified From legend: Not specified title: Not specified Role: group Invalid user entry: false - Accessibility snapshot of <fieldset> after this fix: Name: "Optional Plan Components" aria-labelledby: Not specified aria-label: Not specified From legend: legend.col-form-label "Optional Plan Components" title: Not specified Role: group Invalid user entry: false Labeled by: legend.col-form-label
…le-of-select-tags-and-plan-download-layout Adjustments to style of select tags and plan download layout
… and removed logo_url and identifier_prefix for Shibboleth (as it was causing issues with SSO). Co-authored-by: dstuckey-uoe <[email protected]>
…eds-file Updated seeds.rb file for identifier_schemes to include context value and removed logo_url and identifier_prefix for Shibboleth (as it was causing issues with SSO). It passes the Rspec tests. Co-authored-by: dstuckey-uoe [email protected]
By default, Devise's `:confirmable` module generates a `confirmation_token` and sends confirmation instructions when a new user is created. This commit enhances that behaviour to streamline the email confirmation process for **existing** users. A new rake task (`lib/tasks/email_confirmation.rake#clear_all`) resets the following confirmation-related fields—`confirmed_at`, `confirmation_token`, and `confirmation_sent_at`—to `nil` for all non-superusers. After this reset, these users will be unable to sign in until they confirm their email. To avoid requiring manual re-sending of confirmation instructions, we introduce a new check: `User#confirmed_or_has_confirmation_token?`, which returns `false` if a user is unconfirmed *and* has no outstanding confirmation_token. In the `SessionsController#create` method, if a signing-in user fails the `confirmed_or_has_confirmation_token?` check, we invoke `handle_missing_confirmation_instructions(user)`. This generates a new confirmation_token and sends email instructions. On subsequent sign-in attempts, the check will return `true`, preventing redundant emails. This approach ensures that email confirmations are triggered automatically and only once per affected user, minimising friction while preserving security.
Moved the creation of the Shibboleth identifier to a helper method
- Replaced double-negative `unless existing_user.nil?` with `if existing_user.present?` - Used string concatenation with backslash to allow removal of disabled rubocop offence.
`config/environments/test.rb`
- Add "SMTP From address" to mock Devise's sending of confirmation instructions email at time of account creation
`spec/factories/users.rb`
- Set `confirmed_at { Time.current }` in user factory
`spec/features/registrations_spec.rb`
- Change expected path to `root_path`. This is because the user should not be able to access `plans_path` until after they confirm their email
- Add check to verify that the confirmation-related flash message was sent
- Customises Devise's default value for the key `devise.failure.unconfirmed`. The value adds a link to the email confirmation page. - Updated via `bundle exec rake translation:sync`
Commit 6af91b6 streamlined the email confirmation process for unconfirmed users with no confirmation_token. Specifically, it targeted users attempting to sign in via the app's standard sign-in form. This change applies the same streamlining to users attempting to sign in via SSO/Shibboleth. Specifically, when a user attempts to sign in this way, the `.confirmed_or_has_confirmation_token?` is performed on them. If it returns false, then the confirmation_token is generated for the user and they are auto-sent a confirmation instructions email. (Refer to commit 6af91b6 for more regarding this streamlined process.)
This change addresses the duplicate code shared between `app/controllers/sessions_controller.rb` and `app/controllers/users/omniauth_callbacks_controller.rb`. It does so by creating `app/models/concerns/email_confirmation_handler.rb` which allows us to better adhere to DRY principles. - Additionally, the `user.confirmed? || user.confirmation_token.present?` check has been moved from the User model to the concern. It made sense to have this check as a User method. However, the method itself is simply an or check of two other User methods, and only the `EmailConfirmationHandler` module is currently needing it.
This change maintains the functionality of `def handle_omniauth(scheme)` while making the code more maintainable and addressing its disabled rubocop offences. The private methods `def handle_omniauth_for_signed_in_user(user, scheme)` and `def handle_omniauth_for_signed_out_user(user, scheme)` have been created to handle the omniauth logic for signed in vs signed out users respectively. This change is simply a refactor and maintains the pre-existing code logic.
Commit fdb54e03bb71ffaad17bea562e0ca8d87b081059 resolved the rubocop offences within `def handle_omniauth(scheme)`. However, the created helper methods are now triggering rubocop offences as well. Although, I'm hesitant to simply disable these offences, I also don't want to go overboard with refactoring this file.
- Created `spec/support/helpers/omniauth_helper.rb` for simulating authentication with providers like Shibboleth - Added OmniAuth test config settings in `spec/rails_helper.rb`.
- Tests the custom flow for both system and SSO sign in - Tests the custom flash messages including the embedded link to the confirmation email request page
This change attempts to resolve the following error:
```
1) Email Confirmation A user attempts to sign in via the "Sign in with your institutional credentials"
button. The email is linked to a user account, however the account is
unconfirmed and has no confirmation token.
Failure/Error: raise ActionNotFound.new("The action '#{action}' could not be found for #{self.class.name}", self, action)
AbstractController::ActionNotFound:
The action 'shibboleth' could not be found for Users::OmniauthCallbacksController
```
The Users::OmniauthCallbacksController#shibboleth action is defined dynamically within the controller. Everything is working locally with commit bcc8d1f, but the test is failing when executed as a GitHub Action.
This change explicitly defines the shibboleth-related Users::OmniauthCallbacksController action
The previous configuration had a hardcoded 'en' locale with a comment stating "Keep this as :en. Faker doesn't have :en-GB". However, testing confirms that Faker now works properly with :"en-GB". This change should allow for more accurate testing by using the application's specified locale.
- The previous comments mistakenly stated that the user had no confirmation token.
…firmation Re-Implement Email Confirmation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes # .
Changes proposed in this PR: