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

Adds support for multiple messages #216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zedtux
Copy link

@zedtux zedtux commented Feb 4, 2025

This PR allow one to pass a single message or a list of messages to the fail and success methods.

This will allow the Device gem to pass those messages to warden, and get them displayed on the login pages.

Our use case is the following : We are adding a "delayable" strategy which adds exponential delay between login failures with an information message, and when a single login try remain, we want to show the alert telling about the delay before the user will be able to retry to login, and the alert that only one try remain before his account is being locked.

This PR also adds an Earthlyfile and a docker-compose.yml file for convinience to run Rspec with Docker and Earthly with a single command:

earthly --allow-privileged +rspec

I can even update your Github workflow in order to use Earthly so that we truly benefit from a complete isolation avoiding to get tests failing on the CI, but not locally since Earthly runs Docker in Docker to the tests.

@zedtux zedtux force-pushed the features/support-multiple-messages branch from f9fef63 to b14a97c Compare February 5, 2025 12:46
@jsmestad
Copy link
Collaborator

jsmestad commented Feb 6, 2025

Could you link me to the new devise functionality you're referring to?

@zedtux
Copy link
Author

zedtux commented Feb 6, 2025

Sorry, I have maybe badly explained, but there’s no new Devise functionality so far, this PR introducing multiple messages will allow Devise to support multiple messages too.

Nowadays the way Devise manages messages is by monkey patching the Models inactive_message and unauthenticated_message doing a check and use its message symbol, otherwise call super.

This implementation is a code smell to me, plus it doesn’t allow to have 2 alerts at the same time.

In our use case we need to be able to show 2 alerts from the same request.

I’m preparing a PR for Devise which will use the code from this PR to adapt Devise to send messages, removing the monkey patching and allow models to add many messages.

Do not hesitate if you have any questions.

@zedtux zedtux force-pushed the features/support-multiple-messages branch 4 times, most recently from db1bd4e to baf7b14 Compare February 7, 2025 13:45
@zedtux
Copy link
Author

zedtux commented Feb 10, 2025

@jsmestad I have open my PR on the devise project, hopefully this clarifies even more what we are trying to achieve.

Could you please share your input about this change?

This commit allow one to pass one or many messages to fail or success methods
@zedtux zedtux force-pushed the features/support-multiple-messages branch from baf7b14 to f82a858 Compare February 11, 2025 09:15
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