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

Onboarding gotchas #1

Open
AxelTheGerman opened this issue Jan 7, 2024 · 1 comment
Open

Onboarding gotchas #1

AxelTheGerman opened this issue Jan 7, 2024 · 1 comment

Comments

@AxelTheGerman
Copy link

Hi Jonathan,

Thanks for tackling this web push topic it really is quite a beast, hence DHH thinks it's worth going after!

Just wanted to let you know a couple of gotchas I ran into when installing the gem.

1. DB migration

My users table has UUIDs as primary keys and I had to change that in the migration - common issue and actually happened on noticed itself as well.

2. installer

It failed but I will break out the real issues separately.
$ rails g noticed:web_push:install
2024-01-06T03:09:24.983Z pid=34373 tid=143l INFO: Sidekiq 7.2.0 connecting to Redis with options {:size=>10, :pool_name=>"internal", :url=>nil}
         run  yarn add github:jbennett/noticed-web_push from "."
yarn add v1.22.21
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ [email protected]
info All dependencies
└─ [email protected]
Done in 1.64s.
      append  app/javascript/application.js
         run  touch app/javascript/service_worker.js from "."
      append  app/javascript/service_worker.js
      append  config/initializers/assets.rb
/home/axel/.gem/ruby/3.2.2/bundler/gems/noticed-web_push-132116a5c22b/lib/generators/noticed/web_push/templates/app.webmanifest.erb.tt:8:in `template': undefined method `image_path' for #<Noticed::WebPush::Generators::InstallGenerator:0x00007fb29cae6090 @behavior=:invoke, @_invocations={Noticed::WebPush::Generators::InstallGenerator=>["add_javascript_deps", "setup_service_worker", "create_webmanifest"]}, @_initializer=[[], [], {:behavior=>:invoke, :destination_root=>#<Pathname:/home/axel/src/buzz-me-in>, :shell=>#<Thor::Shell::Color:0x00007fb29c08c6d0 @base=#<Noticed::WebPush::Generators::InstallGenerator:0x00007fb29cae6090 ...>, @mute=false, @padding=0, @always_force=false>}], @options={"skip_namespace"=>false, "skip_collision_check"=>false}, @args=[], @shell=#<Thor::Shell::Color:0x00007fb29c08c6d0 @base=#<Noticed::WebPush::Generators::InstallGenerator:0x00007fb29cae6090 ...>, @mute=false, @padding=0, @always_force=false>, @destination_stack=["/home/axel/src/buzz-me-in"], @indentation=0, @source_paths=["/home/axel/src/buzz-me-in/lib/templates/noticed/install", "/home/axel/.gem/ruby/3.2.2/bundler/gems/noticed-web_push-132116a5c22b/lib/generators/noticed/web_push/templates"], @output_buffer="{\n  \"name\": \"OVERRIDE VIEW IN APPLICATION\",\n  \"short_name\": \"SHORT NAME\",\n  \"start_url\": \"/sign_in\",\n  \"display\": \"fullscreen\",\n  \"icons\": [\n    {\n      \"src\": \""> (NoMethodError)
        from /home/axel/.rubies/ruby-3.2.2/lib/ruby/3.2.0/erb.rb:429:in `eval'
        from /home/axel/.rubies/ruby-3.2.2/lib/ruby/3.2.0/erb.rb:429:in `result'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/file_manipulation.rb:129:in `block in template'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/create_file.rb:54:in `render'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/create_file.rb:64:in `block (2 levels) in invoke!'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/create_file.rb:64:in `open'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/create_file.rb:64:in `block in invoke!'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/empty_directory.rb:117:in `invoke_with_conflict_check'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/create_file.rb:61:in `invoke!'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions.rb:93:in `action'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/create_file.rb:25:in `create_file'
        from /home/axel/.gem/ruby/3.2.2/gems/thor-1.3.0/lib/thor/actions/file_manipulation.rb:125:in `template'
        from /home/axel/.gem/ruby/3.2.2/bundler/gems/noticed-web_push-132116a5c22b/lib/generators/noticed/web_push/install_generator.rb:35:in `create_webmanifest'

So I had to do the following manually:

  • copy the has_many to users
  • add the mounting to routes.rb
  • add tags and buttons to layout

3. yarn

As I'm not using yarn to pull in your scripts, I copied the files from https://github.com/jbennett/noticed-web_push/tree/main/app/assets/javascripts/noticed/web_push to my own app/javascript folder.

I'm not a JS guy so not sure how this can be packaged better - or if it needs to, but I'm using importmaps which is kind of the new default for Rails, so should somehow work with this (and maybe have an option in the installer OR detect whether yarn/npm is used).

Also had to change the imports in the 2 JS files from from "noticed-web_push/web_push" to `from "./web_push/web_push" to resolve to my local copies.

4. Active record encryption

Getting the following error when trying to save the web push subscription:

ActiveRecord::Encryption::Errors::Configuration (Missing Active Record encryption credential: active_record_encryption.deterministic_key):

Makes sense to save this encrypted but if it is a requirement it could maybe be checked in the install script - or it could be made optional (though compromising on security is not good).

Noticed does require rails but only 5.2+ I think, so technically this gem might have to require a more recent rails version itself.

5. ApplicationController

It is weird as it seems you have Noticed::WebPush::ApplicationController but it seems like the controllers are inheriting from my apps ApplicationController as I do have a before_action :require_logged_in_user! which I selectively skip where applicable and it catches for all the gem mounted controller actions.

6. Javascript, again

I actually had to also copy the service_worker.js (copied from the gem) content into my sevice_worker.js file as, at least with import maps, I can't just have an "import" statement.

Again I like the plug & play approach but some of that JS might want to be customized as well - perhaps web_push.start is not the most elegant API/DSL for this.

7. save vs setup subscription

I had to change the Notification.permission == "granted" case to check for existing service worker registration.

            navigator.serviceWorker.getRegistrations().then(registrations => {
              if (registrations.length) {
                saveSubscription()
              } else {
                setupSubscription()
              }
            })

As I hit some snags setting everything up (issues with my service worker especially) it happened that Notification access was granted but the service worker was never set up - and saveSubscription would "hang" forever on const registration = await navigator.serviceWorker.ready

8. I would like to customize the "Enable notifications" button

It relies on it's ID so I can probably just add a button with that ID anywhere I like though.

What I really liked though was:

  • no forcing local certs - I'd rather use a tunnel like ngrok
  • vapid generator task that let me copy&paste the keys

That was a lot and I might have forgotten some!

But I got it working 🚀 Now I still want to clean things up a little bit. It's quite tricky as there are so many moving pieces especially with the JS and all the routes - I also don't know of any clean ways to pass configuration such as routes from Ruby to JS (without dragging them through the HTML).

@jbennett
Copy link
Owner

Thanks so much for your input. I'll be working through this as I think about v2 in light of the stuff that DHH is proposing for Rails 8.

I'm currently thinking about splitting this into two a PWA focused engine that will be conceptually consistent with what the Rails 8 changes will be and a noticed-web_push that builds on that for noticed. I see the first as being a stopgap and will go away once you are on Rails 8.

What are your feelings on that?

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

No branches or pull requests

2 participants