Two different naming convention for Concerns

[heliocola]

Current release of the gem has 2 different naming and filename convention for modules inhering from ActiveSupport::Concern.

1. First naming convention:

• Folder: lib/devise/controllers/concerns
• Filename does not include concern in it
• Module name also does not include Concern in it.

Examples:

• Devise::Passkeys::Controllers::Concerns::PasskeyReauthentication
• Devise::Passkeys::Controllers::Concerns::ReauthenticationChallenge

2. Second naming convention: with concern

• Folder: lib/devise/controllers
• Filename does ends with _concern.rb
• Module name also ends with Concern

Examples:

• Devise::Passkeys::Controllers::PasskeysControllerConcern
• Devise::Passkeys::Controllers::ReauthenticationControllerConcern
• Devise::Passkeys::Controllers::RegistrationsControllerConcern
• Devise::Passkeys::Controllers::SessionsControllerConcern

Other naming inconsistencies

• Some modules repeat Passkey (singular and plural) Devise::Passkeys::Controllers::Concerns::PasskeyReauthentication Devise::Passkeys::Controllers::PasskeysControllerConcern

Is there any good reason for the differentiation? And also: for questions like this, should I create an issue or start a discussion? :-)

[tcannonfodder]

This is definitely a case for a GitHub issue; also whoops 🥴

This definitely needs fixed up

Definitely open to feedback, but my inclination is for naming convention 1. The reason being:

• It keeps all the concerns grouped together under the Concerns:: namespace
• It doesn’t muddy up the module names themselves with the Concern suffix

Some modules repeat Passkey (singular and plural)

The naming convention should standardize on using plurals (because the convention for controllers is that their subjects are pluralized). We should keep Passkeys in the module name itself to help clarify which resource the concern is for

---

So, to recap, the naming convention should be:

Devise::Passkeys::Controllers::Concerns::PasskeysReauthentication

[heliocola]

@tcannonfodder : I agree #1 is better and the plural. How about the duplicated Passkeys in the example you gave above? Do you want to name it PasskeysReauthentication or just Reauthentication?

There is already a Passkeys top level under Devise, I don't see the need for repeating it at the last one in the chain. So your example above, in my view, it would be: Devise::Passkeys::Controllers::Concerns::Reauthentication Instead of: Devise::Passkeys::Controllers::Concerns::PasskeysReauthentication

Also, this change will break compatibility with the current version, and it will affect the devise-passkeys-template app. Do you want to phase this out nicely with deprecation warnings? Or is It too early in the lifetime of the gem? Rubygems shows a total of 189 downloads, some of them I am sure it was me! :-D

[tcannonfodder]

Do you want to name it PasskeysReauthentication or just Reauthentication?

Hmmmm…this one is tricky. While I see your point about it being duplicated; I also like PasskeysReauthentication as the name of the module because it helps when scanning the code to understand what's happening. However, the namespace is already long enough (🥴), and we're quickly veering into over-verbosity here.

So, I the verdict is: drop the Passkeys prefix in the module name, ending up with modules named as you outlined:

Devise::Passkeys::Controllers::Concerns::Reauthentication

Also, this change will break compatibility with the current version, and it will affect the devise-passkeys-template app.

Given that the gem is still in pre-release, we'll:

• Cut a new minor version of the gem (likely 0.3.0
• Add a CHANGELOG.md

We'll also update the template repo. I need to make changes to the template repo's documentation as well. I learned more about how puma-dev works, making the esoteric setup I had before irrelevant.

[johalloran01]

Hey! Working with @tcannonfodder and we agreed this would be a good first issue for me to tackle

[tcannonfodder]

Closed in #7

[heliocola]

@tcannonfodder : I had another question about naming convention that were related to location of files. Currently, after PR #7 we have concerns in the controllers top level folder and in the controllers/concerns.

Just making sure you noticed that. They are all concerns and extend ActiveSupport::Concern. The ones in the top level folder

Should these all be moved to controllers/concerns and remove the _concern from the file name and module name?

[tcannonfodder]

Ah, I did notice that!

So, those actually should remain where they are, because they're what users will include in their application code to get the passkeys functionality.

Pulled from the README:

class Users::SessionsController < Devise::SessionsController
  include Devise::Passkeys::Controllers::SessionsControllerConcern
  # ... any custom code you need

  def relying_party
     WebAuthn::RelyingParty.new(...)
  end

  def set_relying_party_in_request_env
    request.env[relying_party_key] = relying_party
  end
end

In essence, the Devise::Passkeys::Controllers::Concerns::* namespace is for:

Code, related to the concerns for controllers, that can be extracted into a standalone module that can be included & extended as needed for apps that need to do something custom with their setup.

Did that help clear it up? I can see the confusion here; but I'm not exactly sure how we'd namespace these 2 modules without creating even more confusion, so open to ideas.

[heliocola]

@tcannonfodder : thank for the explanation and apologies for the delayed response. I understand now the reason behind it.

I believe we are in the right track to make this more clear: documentation! I will try to push today the docs I was working a few weeks ago. I think one of them is a good place to add these wordings.