391: Use importmaps and hotwire #395

mikevallano · 2023-12-08T13:48:52Z

Related Issues & PRs

Closes #391

Problems Solved

Uses hotwire with turbo_frames and turbo_streams, and uses importmaps for the modern way of handling things

Screenshots

rails-hotwire-in-production.mov

Things Learned

😅

turbo_streams are very similar to how we were handling things back in the day
it's a bit of a pain in the butt to have to worry about all the links that are inside a turbo_frame because they need to have additional data attributes.
the above might be less of a pain in the butt with better organized code, which leads me to:
our code, while a monumental achievement for our budding skills, is overly complex and as we discussed, we should 💣 it up and redo the frontend.
i had a hell of a time trying to use the old style of sprockets asset pipeline to include a single js file.

mikevallano · 2023-12-11T18:41:08Z

.prettierignore

@@ -0,0 +1 @@
+*.scss


I created a separate ticket to use prettier to clean up css files, if we want to: #396

mikevallano · 2023-12-11T18:50:57Z

config/initializers/devise.rb

+  config.responder.error_status = :unprocessable_entity
+  config.responder.redirect_status = :see_other


These are recommended changes as part of the Devise upgrade: https://github.com/heartcombo/devise/blob/main/CHANGELOG.md#490---2023-02-17

mikevallano · 2023-12-11T18:51:37Z

config/routes.rb

-  get 'movies/modal', to: 'movies#modal', as: :movie_modal
+  post 'movies/modal', to: 'movies#modal', as: :movie_modal


turbo_streams can't be used with get requests, so this is why this is changed to a post.

huh. interesting. i guess i hadn't thought about that, but it makes sense.

mikevallano · 2023-12-11T18:52:31Z

spec/support/feature_helpers.rb

+    sleep 0.25
    fill_in "user_login", with: user.email
    find("#user_password")
    fill_in "user_password", with: user.password
    click_button "log_in_button_new_session"
+    sleep 0.25


Unsure why these sleeps are needed now, but it solved the problem 🤷

it might be that waiting for hotwire thing i am about to mention in a comment below... 😆

mikevallano · 2023-12-11T18:53:23Z

spec/support/wait_for_ajax.rb

+    max_time = Capybara::Helpers.monotonic_time + Capybara.default_max_wait_time
+    while Capybara::Helpers.monotonic_time < max_time
+      finished = finished_all_ajax_requests?
+      if finished
+        break
+      else
+        sleep 0.1
+      end
    end
+    raise 'wait_for_ajax timeout' unless finished


Was googling on a more updated way of handling this, and found this code, which was basically copied and pasted

On a related note, at work we discovered a hack where we wait for the hotwire to connect before doing the next cypress action. Just throwing that idea out there in case it becomes relevant to our interests

oooh, interesting. i did a quick googling and didn't find anything. but i will def keep that in mind for the frontend redo since all the frontend tests are gonna need to be addressed.

mikevallano · 2023-12-12T16:27:07Z

config/cable.yml

+  adapter: redis
+  url: redis://localhost:6379/1


auto-added by rails

mikevallano · 2023-12-12T16:28:11Z

package.json

+{
+  "engines": {
+    "node": "20.9.0",
+    "yarn": "1.22.19"
+  }
+}


this was due to warnings from heroku as well as fighting with asset pipeline stuff.

TLDR: it specifies a version of these to be used, even though we're not using them.

mikevallano · 2023-12-12T16:35:59Z

app/controllers/listings_controller.rb

@@ -25,16 +25,16 @@ def create

  def update
    @listing = Listing.find_by("list_id = ? AND movie_id = ?", params[:list_id], params[:movie_id])
+    # TODO: refactor to be more efficient


ticket for refactoring: #400

mikevallano · 2023-12-12T18:11:05Z

app/assets/javascripts/application.js

This file is no longer needed with the Rails 7 way of using importmaps

mikevallano · 2023-12-12T18:11:44Z

app/assets/javascripts/z.js

The bits of this that we still need were moved to autocomplete.js (named that way because it only deals with autocmoplete)

mikevallano · 2023-12-14T19:15:51Z

app/views/ratings/create.turbo_stream.erb

+<%= turbo_stream.update(
+      "movie_rated_#{@movie.tmdb_id}",
+      partial: 'movies/movie_rating',
+      locals: { movie: @movie }
+    )
+%>
+<%= turbo_stream.update(
+      "movie_show_rated_#{@movie.tmdb_id}",
+      partial: 'movies/movie_rating',
+      locals: { movie: @movie }
+    )
+%>


This is a result of us having complexity around showing things in the modal vs show page, for example

lortza · 2023-12-15T00:29:20Z

.gitignore

@@ -38,3 +38,6 @@ coverage/

 # Ignore master key for decrypting credentials and more.
 /config/master.key
+config/credentials.yml.enc


This config/credentials.yml.enc file is not supposed to be in the .gitignore file. If it is, I will not have access to the credentials within. This file works in conjunction with the master.key file (which should be in the gitignore and never committed). Rails uses the master.key to decrypt the credentials file so i can access the values stored within, such as api tokens, etc.

oooooh, got it. i removed it from the gitignore. thanks!

lortza · 2023-12-15T00:30:15Z

Gemfile

+gem 'hotwire-rails'
+gem 'importmap-rails'


lortza · 2023-12-15T00:32:42Z

app/assets/stylesheets/layouts.scss

@@ -16,3 +16,20 @@
  width: 250px;
  margin-left: 40px;
 }
+
+// SIGN OUT LINK NEEDS TO BE A BUTTON_TO. STYLES MATCH BOOTSTRAP
+.navbar #sign_out_nav_link {


I see that this style is using an id here instead of a class. Is that just because it is already what is in place? If this is a new style, we should be using a class instead.

I'm also seeing this for other styles, so that ^ comment applies to all of the places we have ids for styles.

yeah, it was already in place. i'll just leave it on this branch because on redoing the frontend, i plan to remove all of these ids that i believe were added for tests.

lortza · 2023-12-15T00:35:31Z

app/controllers/listings_controller.rb

@@ -13,7 +13,7 @@ def create

    respond_to do |format|
      if listing.save
-        format.js {}
+        format.turbo_stream
        format.html { redirect_to user_list_path(listing.list.owner, listing.list), notice: 'added to your list.' }
        format.json { render :show, status: :created, location: listing }


Just curious: do we need format.json? If not we could delete it. But maybe that's a separate cleanup PR.

Good call. no, we do not use it at all. I created a separate issue to clean that up: #402

lortza · 2023-12-15T00:39:49Z

app/controllers/taggings_controller.rb

@@ -16,10 +16,12 @@ def destroy
    @tagging = current_user.taggings.find_by("tag_id = ? AND movie_id = ?", params[:tag_id], params[:movie_id])
    respond_to do |format|
      if @tagging && @tagging.destroy
-        format.js {}
+        format.turbo_stream {}
+        format.html { redirect_to movie_path(@movie), notice: 'Tag was destroyed.' }


i'm vaguely remembering something from the pragmatic studio tutorial about how we need to handle notices or errors or something in controllers... like we have to specifically declare (:fan: ✋) something now

not a super helpful comment 😆 my brain is sleepy

hmm, yeah, this is a weird one because it'll always be a turbo_stream response. but general handling of notices/errors can be part of the frontend redo.

lortza · 2023-12-15T00:50:48Z

spec/support/wait_for_ajax.rb

+    max_time = Capybara::Helpers.monotonic_time + Capybara.default_max_wait_time
+    while Capybara::Helpers.monotonic_time < max_time
+      finished = finished_all_ajax_requests?
+      if finished
+        break
+      else
+        sleep 0.1
+      end
    end
+    raise 'wait_for_ajax timeout' unless finished


On a related note, at work we discovered a hack where we wait for the hotwire to connect before doing the next cypress action. Just throwing that idea out there in case it becomes relevant to our interests

lortza · 2023-12-15T00:51:45Z

spec/support/feature_helpers.rb

+    sleep 0.25
    fill_in "user_login", with: user.email
    find("#user_password")
    fill_in "user_password", with: user.password
    click_button "log_in_button_new_session"
+    sleep 0.25


it might be that waiting for hotwire thing i am about to mention in a comment below... 😆

lortza · 2023-12-17T14:34:57Z

app/views/layouts/application.html.erb

@@ -11,17 +11,19 @@
    <link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:opsz,wght,FILL,GRAD@48,400,0,0" />

    <%= stylesheet_link_tag    'application', media: 'all'%>
-    <%= javascript_include_tag 'application'%>
+    <%= javascript_include_tag 'application' %>
+    <%= javascript_import_module_tag "autocomplete" %>


😅 thanks for sticking with it and making this work!

lortza · 2023-12-17T14:36:43Z

app/views/layouts/application.html.erb

+    <!--TODO: use importmap for this-->
    <script src="//use.fontawesome.com/16a020f0b8.js"></script>


i have also experienced a lot of bootstrap drama with rails 7 upgrades and general fontawesome unhappiness. so, yeah i'm down to switch to material.

lortza · 2023-12-17T14:44:49Z

config/routes.rb

-  get 'movies/modal', to: 'movies#modal', as: :movie_modal
+  post 'movies/modal', to: 'movies#modal', as: :movie_modal


huh. interesting. i guess i hadn't thought about that, but it makes sense.

lortza · 2023-12-17T14:45:15Z

package.json

+{
+  "engines": {
+    "node": "20.9.0",
+    "yarn": "1.22.19"
+  }
+}


mikevallano added the wip label Dec 8, 2023

mikevallano temporarily deployed to rocky-headland-81947 December 8, 2023 18:00 Inactive

mikevallano temporarily deployed to rocky-headland-81947 December 8, 2023 18:04 Inactive

mikevallano force-pushed the 392-upgrade-to-rails-7-1-2 branch from a39a728 to 1ae9a05 Compare December 8, 2023 18:49

Base automatically changed from 392-upgrade-to-rails-7-1-2 to master December 8, 2023 18:53

mikevallano added 25 commits December 8, 2023 13:56

391: Initial addition of gems

ca12252

391: WIP. Bootstrap is working with jquery now

82359d4

391: Use turbo_stream on taggings

c7a5d5c

391: Clean up form syntax

8e826bc

391: Handle tag deletes

bb4d120

391: WIP: screening somewhat working

1edb93a

391: Fix css for x buttons

eacdbc4

391: Move partial-spacer to partials

aa0fd1e

391: Use update instead of replace. Keep partial-spacers where they were

02404b5

391: Fix styles for button_to on small-x again

9a223bb

391: Add stimuls controller to validate trailer url

0d89306

391: Handle rating a movie. Add auto_submit_controller

0573459

391: Add to another list and auto submit working

615df99

391: Handle deleting listings

6280e83

WIP: Modal nearly working

d0ddcb4

391: Fix up movies not yet in db

0a94bde

391: Autocomplete is mostly working

9ddd53f

391: Make modal link break out of turbo frame

b057f53

391: Comment out unused js

1193100

391: Fix movie poster css. Ignore sass files in prettier

60f9620

391: Get rid of dummy modal code

8ecb950

391: Add attributes to not use turbo when clicking links from modal

dcf3dff

391: Add modal controller and use it over temp testing methods

d86633b

391: Various fixes after doing QA

9bfbb14

391: Update specs

a81c443

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 15:17 Inactive

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 15:21 Inactive

mikevallano force-pushed the 391-updates-after-upgrades branch from edafbdb to 66dfb71 Compare December 12, 2023 15:23

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 15:23 Inactive

mikevallano force-pushed the 391-updates-after-upgrades branch from 66dfb71 to ec853c8 Compare December 12, 2023 15:27

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 15:28 Inactive

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 15:51 Inactive

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 16:04 Inactive

391: Asset management tweaks

b6c5236

mikevallano force-pushed the 391-updates-after-upgrades branch from 426edbd to b6c5236 Compare December 12, 2023 16:12

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 16:12 Inactive

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 16:29 Inactive

mikevallano removed the wip label Dec 12, 2023

mikevallano marked this pull request as ready for review December 12, 2023 16:43

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 16:58 Inactive

mikevallano added 2 commits December 12, 2023 13:04

391: Remove application_legacy and move autocomplete.js

ea3e2dc

391: PR cleanup moar

7fb7601

mikevallano force-pushed the 391-updates-after-upgrades branch from 6014615 to 7fb7601 Compare December 12, 2023 18:05

mikevallano temporarily deployed to rocky-headland-81947 December 12, 2023 18:05 Inactive

mikevallano commented Dec 12, 2023

View reviewed changes

mikevallano commented Dec 14, 2023

View reviewed changes

mikevallano temporarily deployed to rocky-headland-81947 December 14, 2023 23:20 Inactive

lortza reviewed Dec 15, 2023

View reviewed changes

mikevallano temporarily deployed to rocky-headland-81947 December 15, 2023 23:33 Inactive

391: Fix up new screening and tag links

aeeed43

mikevallano temporarily deployed to rocky-headland-81947 December 16, 2023 17:52 Inactive

lortza approved these changes Dec 17, 2023

View reviewed changes

391: Remove credentials.yml.enc from gitignore

8f8377e

mikevallano merged commit 7a91d20 into master Dec 17, 2023

mikevallano deleted the 391-updates-after-upgrades branch December 17, 2023 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

391: Use importmaps and hotwire #395

391: Use importmaps and hotwire #395

mikevallano commented Dec 8, 2023 •

edited

Loading

mikevallano Dec 11, 2023

mikevallano Dec 11, 2023

mikevallano Dec 11, 2023

lortza Dec 17, 2023

mikevallano Dec 11, 2023

lortza Dec 15, 2023

mikevallano Dec 11, 2023

lortza Dec 15, 2023

mikevallano Dec 17, 2023

mikevallano Dec 12, 2023

mikevallano Dec 12, 2023

lortza Dec 17, 2023

mikevallano Dec 12, 2023

mikevallano Dec 12, 2023

mikevallano Dec 12, 2023

mikevallano Dec 14, 2023

lortza Dec 15, 2023 •

edited

Loading

mikevallano Dec 17, 2023

lortza Dec 15, 2023

lortza Dec 15, 2023

mikevallano Dec 17, 2023

lortza Dec 15, 2023

mikevallano Dec 17, 2023

lortza Dec 15, 2023

mikevallano Dec 17, 2023

lortza Dec 15, 2023

lortza Dec 15, 2023

lortza Dec 17, 2023

lortza Dec 17, 2023

lortza Dec 17, 2023

lortza Dec 17, 2023

		config.responder.error_status = :unprocessable_entity
		config.responder.redirect_status = :see_other

		get 'movies/modal', to: 'movies#modal', as: :movie_modal
		post 'movies/modal', to: 'movies#modal', as: :movie_modal

		<!--TODO: use importmap for this-->
		<script src="//use.fontawesome.com/16a020f0b8.js"></script>

		@@ -0,0 +1 @@
		*.scss

		gem 'hotwire-rails'
		gem 'importmap-rails'

391: Use importmaps and hotwire #395

391: Use importmaps and hotwire #395

Conversation

mikevallano commented Dec 8, 2023 • edited Loading

Related Issues & PRs

Problems Solved

Screenshots

Things Learned

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lortza Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikevallano commented Dec 8, 2023 •

edited

Loading

lortza Dec 15, 2023 •

edited

Loading