MushroomObserver / mushroom-observer

A website for sharing observations of mushrooms.
https://mushroomobserver.org
MIT License
77 stars 25 forks source link

Field Slip PDFs #2081

Closed mo-nathan closed 3 months ago

mo-nathan commented 3 months ago

Builds on the SolidQueue PR. Probably makes sense to just keep them merged and deploy them together.

To test:

  1. Run the migrations.
  2. Start bundle exec rake solid_queue:start in a new shell.
  3. Go to a project with a field slip prefix.
  4. Click "Create Field Slip PDF".
  5. Click "Create" (feel free to update the number of Pages.
  6. The jobs should now appear at the top of the table below the "Create" button and should update every second until it is done.
  7. Once it's done, the file name in the display should turn into a link that will download the PDF.
  8. The files and FieldSlipJobTracker objects get cleanup during the first job that gets run after they have not been updated for a week.

See #1949 for more notes on what was implemented, but the above covers most of it.

coveralls commented 3 months ago

Coverage Status

coverage: 94.39% (+0.06%) from 94.335% when pulling 9b6e5c091ab8490cdca4e9ebe7d175828e860202 on njw-field-slip-pdfs into 7a986ee26e92fb214b6fa4c2ee6b0adfabda7f6f on main.

mo-nathan commented 3 months ago

I'm not sure about the service stuff I just added. Specifically in config/etc/solidqueue.service and and script/deploy.sh.

mo-nathan commented 3 months ago

@pellaea @nimmolo Can you review the services bit of this for SolidQueue? I tried following combining what we're doing with Puma with what I found here https://gitlab.com/-/snippets/3693757. Not sure how to test this other than just trying and seeing if it works.

nimmolo commented 3 months ago

One thing i'm noticing in the solid queue docs, I don't know if it's necessary, but it seems like it might put error messages where we're more likely to catch them when we're not already thinking about it:

We provide a Puma plugin if you want to run the Solid Queue's supervisor together with Puma and have Puma monitor and manage it. You just need to add plugin :solid_queue to your puma.rb configuration.

nimmolo commented 3 months ago

I agree that this will be hard to troubleshoot without just trying it out.

mo-nathan commented 3 months ago

One thing i'm noticing in the solid queue docs, I don't know if it's necessary, but it seems like it might put error messages where we're more likely to catch them when we're not already thinking about it:

We provide a Puma plugin if you want to run the Solid Queue's supervisor together with Puma and have Puma monitor and manage it. You just need to add plugin :solid_queue to your puma.rb configuration.

I noticed that as well, but decided not to try it because it creates another dependency (something we have to fix when we switch from Puma to whatever the next Rails webserver is). It seems the main reason this feature exists is to avoid extra billing cost on Heroku which doesn't matter to us.

JoeCohen commented 3 months ago

Manual test threw ArgumentError at step 4 ("In the console, run FieldSlipJob.new.perform(Project.first.id, 0, 6) (or pick your favorite Project)""). I'm sure this is something simple, but don't have time this morning to figure it out.

irb(main):001> FieldSlipJob.new.perform(Project.first.id, 0, 6)
  Project Load (0.3ms)  SELECT `projects`.* FROM `projects` ORDER BY `projects`.`id` ASC LIMIT 1
/Users/joe/mushroom-observer/app/jobs/field_slip_job.rb:6:in `perform': wrong number of arguments (given 3, expected 2) (ArgumentError)
mo-nathan commented 3 months ago

Manual test threw ArgumentError at step 4 ("In the console, run FieldSlipJob.new.perform(Project.first.id, 0, 6) (or pick your favorite Project)""). I'm sure this is something simple, but don't have time this morning to figure it out.

irb(main):001> FieldSlipJob.new.perform(Project.first.id, 0, 6)
  Project Load (0.3ms)  SELECT `projects`.* FROM `projects` ORDER BY `projects`.`id` ASC LIMIT 1
/Users/joe/mushroom-observer/app/jobs/field_slip_job.rb:6:in `perform': wrong number of arguments (given 3, expected 2) (ArgumentError)

Whoops, didn't update the instructions. You no longer need to go into the console. It can all be managed from the UI. See the updated instructions.

JoeCohen commented 3 months ago

Tried again, but running into a conflict with the long/lng change. I'm almost out the door for today soprobalby won't be able to resolve the conflict till tomorrow.

NameError in Observations#index
Showing /Users/joe/mushroom-observer/app/views/controllers/shared/_matrix_box.erb where line #38 raised:

undefined local variable or method `long' for an instance of Observation
Extracted source (around line #798):

    "#{lat.abs}°#{lat.negative? ? "S" : "N"} " \
      "#{long.abs}°#{long.negative? ? "W" : "E"}"
  end

  def display_alt

Trace of template inclusion: #<ActionView::Template app/views/controllers/observations/index.html.erb locals=[]>

Rails.root: /Users/joe/mushroom-observer

Application Trace | Framework Trace | Full Trace
app/models/observation.rb:798:in `display_lat_long'
app/helpers/observations_helper.rb:177:in `observation_details_where_gps'
app/helpers/observations_helper.rb:150:in `observation_details_when_where_who'
app/helpers/lightbox_helper.rb:36:in `lightbox_obs_caption'
app/helpers/lightbox_helper.rb:20:in `lightbox_caption_html'
app/helpers/lightbox_helper.rb:8:in `lightbox_link'
app/helpers/images_helper.rb:45:in `block in interactive_image'
app/helpers/images_helper.rb:31:in `interactive_image'
app/helpers/matrix_box_helper.rb:61:in `block in matrix_box_image'
app/helpers/matrix_box_helper.rb:60:in `matrix_box_image'
app/views/controllers/shared/_matrix_box.erb:38
app/views/controllers/shared/_matrix_box.erb:36
app/views/controllers/shared/_matrix_box.erb:33
app/helpers/matrix_box_helper.rb:52:in `block in matrix_box'
app/helpers/matrix_box_helper.rb:51:in `matrix_box'
app/views/controllers/shared/_matrix_box.erb:32
app/helpers/matrix_box_helper.rb:36:in `block (2 levels) in render_cached_matrix_boxes'
app/helpers/matrix_box_helper.rb:35:in `block in render_cached_matrix_boxes'
app/helpers/matrix_box_helper.rb:34:in `each'
app/helpers/matrix_box_helper.rb:34:in `render_cached_matrix_boxes'
app/helpers/matrix_box_helper.rb:20:in `block in matrix_table'
app/helpers/matrix_box_helper.rb:12:in `matrix_table'
app/views/controllers/observations/index.html.erb:39
app/helpers/pagination_helper.rb:19:in `paginate_block'
app/views/controllers/observations/index.html.erb:38
app/controllers/application_controller.rb:1531:in `show_index_render'
app/controllers/application_controller.rb:1387:in `show_index_of_objects'
app/controllers/observations_controller/index.rb:189:in `show_selected_observations'
app/controllers/observations_controller/index.rb:21:in `list_all'
app/controllers/observations_controller/index.rb:14:in `default_index_subaction'
app/controllers/application_controller.rb:1341:in `index'
app/controllers/application_controller.rb:226:in `catch_errors_and_log_request_stats'
mo-nathan commented 3 months ago
app/models/observation.rb

Looks right to me on the current version of this branch. There were some changes this morning, so maybe you need to do a git pull?

  def display_lat_lng
    return "" unless lat

    "#{lat.abs}°#{lat.negative? ? "S" : "N"} " \
      "#{lng.abs}°#{lng.negative? ? "W" : "E"}"
  end
JoeCohen commented 3 months ago

It created a page of field slips. (I'm at the airport) Possible issue re field slip #s I used the alpha test project from this snapshot database-20240409.gz The field slips are numbered 0-5 Locally the highest field slip is 12 for your Obs 547665

mo-nathan commented 3 months ago

It created a page of field slips. (I'm at the airport) Possible issue re field slip #s I used the alpha test project from this snapshot database-20240409.gz The field slips are numbered 0-5 Locally the highest field slip is 12 for your Obs 547665

Yeah, I need to update the max number for projects that have field slips. Thanks for the reminder.

mo-nathan commented 3 months ago

@nimmolo What's the best way to manage the state updates for the background job using a spinner or a progress bar or some such? See the "Create FIeld Slip PDF" page. Right now it takes you back to the project show page and if you click the button again, it's usually done, but that's not a great UX.

nimmolo commented 3 months ago

Thinking about it. It seems the job (or tracker) needs to fire off a response that can be picked up... by the controller or view template for the "Create PDF" page... by which it can... update the spinner?

I haven't dug into jobs enough to know what these callbacks are. Investigating...

mo-nathan commented 3 months ago

Thinking about it. It seems the job (or tracker) needs to fire off a response that can be picked up... by the controller or view template for the "Create PDF" page... by which it can... update the spinner?

I haven't dug into jobs enough to know what these callbacks are. Investigating...

At the moment there is a status field that goes from 'Starting' to 'Processing' to 'Done'. This gets recorded in the database in the FieldSlipJobTracker object. It would also be possible to record the count of pages that have been processed so far as part of the job. I think what this means is a browser on the Create PDF page needs to periodically (once per second?) make an AJAX call to the backend asking for the status of each job that is not already in the 'Done' state as far as the browser knows. The call would return the status (and the page count if that gets added). The JS code would then update the DOM with any change in status, turn the file name into a link once it gets to 'Done', and potentially updates the page count. For NEMF the plan at the moment is to render 800 pages (4,800 slips) which locally took 135 seconds. At this point I think the most useful thing would be update the "Seconds" column and to add a page count column. Then it would update every second. I've attached the current UI and the HTML from the Chrome inspector. I'm assuming I need to provide id's for each item in the table. Happy to do something bootstrappy for the table if you think that would be significantly better. My concern is I don't want to just go do something old school that there is some newer, better way to do.

Screenshot 2024-04-16 at 7 51 59 AM Screenshot 2024-04-16 at 7 38 04 AM
nimmolo commented 3 months ago

I think i get it - thank you for that explanation.

Most important thing: old school JS is not even an option anymore - it will not load in the page.

Let me know if you want to tackle it, or if you'd like me to do it.

mo-nathan commented 3 months ago

Ok, I'll checkout the controllers you pointed out and see how suck I get. Thanks!

nimmolo commented 3 months ago

Ok. I'm psyched you're going to try it, I think you may enjoy working with Stimulus more than "regular" JS, even though you can't use jQuery in it by default.

I find it reassuringly "conventional over configurational". You don't have to re-invent the fork and the plate for each meal (although of course that can also be part of the fun when camping). Once you get the hang of the few conventions, it makes working with JS a lot more like working with Rails. I honestly think Joe might even like it!

One key thing to remember is: every Stimulus controller has a DOM scope. It only acts on the element it's attached to, and that element's descendants. You're basically attaching a JS "interactive mechanism" to a discrete part of the DOM, and it only watches that part.

(The exception is that other elements in the DOM can be made to fire "Stimulus events" that are picked up by other controllers, if they're instructed to listen for those events.)

mo-nathan commented 3 months ago

Recommendation for the future: Don't point newbies and your biggest, gnarliest controllers. autocompleter_controller.js is over a 1100 lines and obs-form-images_controller.js is over 900, but the median size is 30 lines.

nimmolo commented 3 months ago

Yes, sorry. Point taken. The gnarliest two make the most use of requestjs, I guess is why they came to mind.

nimmolo commented 3 months ago

New PR #2104