Add dummy app by RenzoMinelli · Pull Request #7 · cedarcode/devise-webauthn

RenzoMinelli · 2025-08-10T23:18:07Z

Summary

Add dummy app that will be used to test the gem

joaquintomas2003

I think we should add bin/rails in the dummy app, right?

RenzoMinelli · 2025-08-12T01:05:35Z

I think we should add bin/rails in the dummy app, right?

Why do you think so? I wasn't thinking the dummy app as an executable rails app. More like an app that allows doing some tests with the models we define there.

joaquintomas2003 · 2025-08-12T13:46:40Z

Why do you think so?

I was thinking to be able to run the server, or even run the migrations

RenzoMinelli · 2025-08-13T01:09:27Z

Why do you think so?

I was thinking to be able to run the server, or even run the migrations

Okay I ended up adding a lot more files, I copied the structure from webauthn-rails. Now we can run the server and run the migrations.
Also added some tests to verify that the models in the app got correctly loaded.

``` bundle config set force_ruby_platform true bundle install ```

santiagorodriguez96

Looking great! 🔝

Some small comments in order to keep it moving forward!! 💪

santiagorodriguez96 · 2025-08-20T15:13:54Z

+//= link_tree ../images
+//= link_directory ../stylesheets .css
+//= link_tree ../../javascript .js
+//= link_tree ../../../vendor/javascript .js


We should be able to remove this as we are using propshaft, right?

santiagorodriguez96 · 2025-08-20T15:15:23Z

+/*
+ * This is a manifest file that'll be compiled into application.css, which will include all the files
+ * listed below.
+ *
+ * Any CSS and SCSS file within this directory, lib/assets/stylesheets, vendor/assets/stylesheets,
+ * or any plugin's vendor/assets/stylesheets directory can be referenced here using a relative path.
+ *
+ * You're free to add application-wide styles to this file and they'll appear at the bottom of the
+ * compiled file so the styles you add here take precedence over styles defined in any other CSS/SCSS
+ * files in this directory. Styles in this file should be added after the last require_* statement.
+ * It is generally better to create a new file per style scope.
+ *
+ *= require_tree .
+ *= require_self
+ */


Same here!

Suggested change

/*

* This is a manifest file that'll be compiled into application.css, which will include all the files

* listed below.

*

* Any CSS and SCSS file within this directory, lib/assets/stylesheets, vendor/assets/stylesheets,

* or any plugin's vendor/assets/stylesheets directory can be referenced here using a relative path.

*

* You're free to add application-wide styles to this file and they'll appear at the bottom of the

* compiled file so the styles you add here take precedence over styles defined in any other CSS/SCSS

* files in this directory. Styles in this file should be added after the last require_* statement.

* It is generally better to create a new file per style scope.

*

*= require_tree .

*= require_self

*/

/*

* This is a manifest file that'll be compiled into application.css.

*

* With Propshaft, assets are served efficiently without preprocessing steps. You can still include

* application-wide styles in this file, but keep in mind that CSS precedence will follow the standard

* cascading order, meaning styles declared later in the document or manifest will override earlier ones,

* depending on specificity.

*

* Consider organizing styles into separate files for maintainability.

*/

creating a new plugin I see that the dummy app has an application.css like the one we have now.

Looking into the generators I see there's a difference between the template for the app and the template for the plugin:

https://github.com/rails/rails/blob/cf6ff17e9a3c6c1139040b519a341f55f0be16cf/railties/lib/rails/generators/rails/app/templates/app/assets/stylesheets/application.css.tt

https://github.com/rails/rails/blob/41f4e981ded1c0d8c2b3bc5bff5fb2af38da04ed/railties/lib/rails/generators/rails/plugin/templates/rails/stylesheets.css

https://github.com/rails/rails/blob/41f4e981ded1c0d8c2b3bc5bff5fb2af38da04ed/railties/lib/rails/generators/rails/plugin/plugin_generator.rb#L155-L157

🤔

Ahh that makes sense. I wonder why rails plugin new generates an application.css in the dummy app but does not install any gem to manage the asset pipeline 🤔

That said, if we are installing propshaft on this dummy app I think we should use the application.css that comes in a default Rails 8 app, don't you think?

santiagorodriguez96 · 2025-08-20T15:19:59Z

+import { Turbo } from "@hotwired/turbo-rails"
+import "controllers/application"
+
+Turbo.session.drive = false


Is this needed?

santiagorodriguez96 · 2025-08-20T15:23:11Z

+    <%= csrf_meta_tags %>
+    <%= csp_meta_tag %>
+
+    <%= stylesheet_link_tag "application" %>


As we are using propshaft, we should change this to:

Suggested change

<%= stylesheet_link_tag "application" %>

<%= stylesheet_link_tag :app %>

right?

santiagorodriguez96 · 2025-08-20T15:46:19Z

@@ -1,3 +1,5 @@
+--require rails_helper


Out of curiosity: why did we need to add this? I think in general only the spec_helper is required on this file 🤔

santiagorodriguez96 · 2025-08-20T15:50:02Z

  spec.metadata["rubygems_mfa_required"] = "true"
  spec.required_ruby_version = ">= 3.1"

+  spec.add_development_dependency "byebug"


Thoughs about using pry-byebug?

santiagorodriguez96 · 2025-08-20T15:50:58Z

  spec.add_development_dependency "rubocop-rspec", "~> 3.6"
+  spec.add_development_dependency "sqlite3", "~> 2.7"
+  spec.add_development_dependency "stimulus-rails", "~> 1.3"
+  spec.add_development_dependency "turbo-rails", "~> 2.0"


Out of curiosity: is turbo-rails needed?

santiagorodriguez96 · 2025-08-20T15:52:26Z

+
+Style/NumericLiterals:
+  Exclude:
+    - 'spec/dummy/db/schema.rb'


Should we exclude this file from the dummy from all Rubocop's cops? Should we exclude all the files of the dummy app from Rubocop's cops?

santiagorodriguez96 · 2025-08-20T15:56:05Z

+  it "allows building and saving a passkey" do
+    user = User.new(email: "test@example.com", password: "password")
+    passkey = described_class.new(name: "test", external_id: "external_id_123", public_key: "public_key_123",
+                                  sign_count: 0, user:)
+
+    expect(passkey.save).to be_truthy
+
+    passkey.reload
+
+    expect(passkey).to be_persisted
+  end
+end


What are we trying to test here? Seems to me that we are testing ActiveRecord's validations and persistance methods, which feels unnecessary to me.

What do you all think?

I was simply adding some test to verify that the models were loaded correctly and the database changes worked as well. So simple tests for that

Makes sense! What do you think about skipping the tests for now and add them as part of a different PR?

joaquintomas2003 · 2025-08-21T16:26:52Z

Should we close this PR in favor of this one: #10?

And open separate PRs for the things that are out of scope :)

RenzoMinelli force-pushed the rm/add-dummy-app branch 3 times, most recently from 429fc83 to ffa8a36 Compare August 11, 2025 00:47

RenzoMinelli marked this pull request as ready for review August 11, 2025 00:48

joaquintomas2003 reviewed Aug 11, 2025

View reviewed changes

Comment thread spec/dummy/db/schema.rb Outdated

RenzoMinelli force-pushed the rm/add-dummy-app branch from ffa8a36 to 41d58b6 Compare August 12, 2025 01:25

Base automatically changed from rm/add-devise-strategy to master August 12, 2025 18:48

RenzoMinelli added 5 commits August 15, 2025 22:35

add dummy app

108e750

remove unnecessary migration

e3b6a66

load schema on tests

94eb8ff

add test to verify user and passkey are loaded

67a86f9

add missing dummy app parts

1c072f5

RenzoMinelli force-pushed the rm/add-dummy-app branch from 76384ec to 1c072f5 Compare August 16, 2025 01:36

RenzoMinelli and others added 8 commits August 16, 2025 15:04

fix outdated schema

ee26c61

add missing module and fix sign out

78e366a

fix assets issues

17b7455

Stop git ignoring spec/dummy/vendor

2d363e5

Add stimulus-rails

5fb798b

Add turbo-rails

6979782

Fix stimulus and turbo setups

3ba12b8

build(deps): Force ruby platform

7856433

``` bundle config set force_ruby_platform true bundle install ```

santiagorodriguez96 reviewed Aug 20, 2025

View reviewed changes

RenzoMinelli closed this Aug 21, 2025

joaquintomas2003 deleted the rm/add-dummy-app branch October 21, 2025 15:49

	<%= stylesheet_link_tag "application" %>
	<%= stylesheet_link_tag :app %>

Conversation

RenzoMinelli commented Aug 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

joaquintomas2003 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RenzoMinelli commented Aug 12, 2025

Uh oh!

joaquintomas2003 commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RenzoMinelli commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

santiagorodriguez96 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joaquintomas2003 commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RenzoMinelli commented Aug 10, 2025 •

edited

Loading

joaquintomas2003 commented Aug 12, 2025 •

edited

Loading

RenzoMinelli commented Aug 13, 2025 •

edited

Loading