Create Obs form phase 1: images and EXIF data

[nimmolo]

Revamp of the obs edit/create form image section.

Part 1 of 3.

[coveralls]

coverage: 94.393% (+0.001%) from 94.392% when pulling 0d5be176ef8e65817d844aa69376701e81f250e6 on create-obs-reorg into 707064b564ef4c4ae30513e255c5f884257a4642 on main.

[mo-nathan]

Looking at the latest on this branch, I find the initial layout of the “Select file…” button to be confusing and generally not great. If I haven’t selected any images, I’d rather just see a “Select Images” button and not an empty panel with the title “Images” and a button way over on the right hand side of the screen.

(Maybe I wasn't supposed to look at the latest on this branch since I see now that test.mushroomobserver.org doesn't look like that.)

[mo-nathan]

Hmm, I tried running it and the large images aren't showing up. The smaller images along the bottom are (when there's more than one of them).

At first I thought it might be due to SolidQueue not running, but I started it up and I'm seeing the same problem.

[JoeCohen]

I thought this was on the test server. Am I wrong?

On Fri, Jun 14, 2024 at 4:53 PM Nathan Wilson @.***> wrote:

Hmm, I tried running it and the large images aren't showing up. The smaller images along the bottom are (when there's more than one of them). Screenshot.2024-06-14.at.7.49.03.PM.png (view on web) https://github.com/MushroomObserver/mushroom-observer/assets/3211730/82511414-5674-4204-9567-4f84d9aa0adc Screenshot.2024-06-14.at.7.47.56.PM.png (view on web) https://github.com/MushroomObserver/mushroom-observer/assets/3211730/a349167b-8380-42a7-9ea6-6aa74fd45e68

At first I thought it might be due to SolidQueue not running, but I started it up and I'm seeing the same problem.

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2155#issuecomment-2168931915, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDFEDCSRAGA3NBVONIMLZHN7ANAVCNFSM6AAAAABIQFB42KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRYHEZTCOJRGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[mo-nathan]

Why do I have to run it on the test server? That seems wrong.

[mo-nathan]

test.mushroomobserver.org does work better.

Initial Comments: 1) I think the first field being "Notes" is kind of odd. It's not immediately clear if this is notes about the image or the observation, Given that observation notes can be more structured I think this needs to be clarified. Assuming they are "Image Notes", I would label them as such and I'd put them at after the "Camera Info". 2) I think there should be an option for creating the observation above the fold. 3) Just adding the images and then clicking "Create" caused an observation to be created with no images.

[JoeCohen]

@nathan: I don't think you have to run it on the test server. But for me, it's easier than switching branches.

On the test server, type-ahead did not work (for either Location or Scientific Name).

The lat/lng and date autofills work great. Incomplete thinking out loud: Since those will be auto-filled in most cases, would it make sense to put them after the fields which always require manual input?

I agree with Nathan about clarifying the image Notes field.

@nathan does "option for creating the observation above the fold" mean you want an additional Create button? Not a bad idea. Maybe (at a later point) instead have a single floating button if that's easy.

On Fri, Jun 14, 2024 at 6:39 PM Nathan Wilson @.***> wrote:

test.mushroomobserver.org does work better.

Initial Comments:

1. I think the first field being "Notes" is kind of odd. It's not immediately clear if this is notes about the image or the observation, Given that observation notes can be more structured I think this needs to be clarified. Assuming they are "Image Notes", I would label them as such and I'd put them at after the "Camera Info".
2. I think there should be an option for creating the observation above the fold.
3. Just adding the images and then clicking "Create" caused an observation to be created with no images.

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2155#issuecomment-2169029319, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDFBKZOHQVKTNTM2UCLDZHOLOZAVCNFSM6AAAAABIQFB42KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRZGAZDSMZRHE . You are receiving this because you commented.Message ID: @.***>

[JoeCohen]

PS: It created an Observation without images, even though the form had 2 images (and the controller grabbed date and lat/lng from them).

On Fri, Jun 14, 2024 at 7:23 PM Joseph Cohen @.***> wrote:

@nathan: I don't think you have to run it on the test server. But for me, it's easier than switching branches.

On the test server, type-ahead did not work (for either Location or Scientific Name).

The lat/lng and date autofills work great. Incomplete thinking out loud: Since those will be auto-filled in most cases, would it make sense to put them after the fields which always require manual input?

I agree with Nathan about clarifying the image Notes field.

@nathan does "option for creating the observation above the fold" mean you want an additional Create button? Not a bad idea. Maybe (at a later point) instead have a single floating button if that's easy.

On Fri, Jun 14, 2024 at 6:39 PM Nathan Wilson @.***> wrote:

test.mushroomobserver.org does work better.

Initial Comments:

1. I think the first field being "Notes" is kind of odd. It's not immediately clear if this is notes about the image or the observation, Given that observation notes can be more structured I think this needs to be clarified. Assuming they are "Image Notes", I would label them as such and I'd put them at after the "Camera Info".
2. I think there should be an option for creating the observation above the fold.
3. Just adding the images and then clicking "Create" caused an observation to be created with no images.

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2155#issuecomment-2169029319, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDFBKZOHQVKTNTM2UCLDZHOLOZAVCNFSM6AAAAABIQFB42KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRZGAZDSMZRHE . You are receiving this because you commented.Message ID: @.***>

[nimmolo]

@mo-nathan - The fact that the new form is letting you create an obs w/o a location sounds like a validation failure. I didn't even think to check it because i don't remember changing anything in the validation methods of the create action. Thank you.

The screengrabs from your local machine tell me your scss is not compiling. There may be a deeper problem with dartsass-sprockets, but it's more likely you have old assets lingering.

You may know this, but as a rule of thumb: We should only precompile assets on production. Try to remember never to run assets:precompile locally, without running assets:clobber immediately afterward to remove the compiled assets, or you will get stuck in a CSS time warp, having to manually precompile every time you want an upstream change to be reflected locally.

The presence of any files in public/assets (from precompiling) is a signal to the sass compiler that will block live SCSS compilation on your machine indefinitely, because these folders are gitignored! Maybe also check for app/assets/builds. Removing these folders, or running assets:clobber, is your Tardis back to the present.

And getting freshly-compiled CSS on dev will become increasingly relevant in the near future while we are updating all the CSS.

@JoeCohen I haven't been able to replicate the obs being created with the images getting lost, but on test, i do get to the obs show page sometimes before the images have been transferred. This makes them appear blank at first. On reload, for me they are there.

[nimmolo]

I should have pointed this out — note that observations created on the test server are also created in the production database, so if you don't want to keep your experiments, you may want to delete them.

@JoeCohen if this is the obs you tried to create with images, where your images were not attached to the obs, I think i found it. They sure aren't there, are they? 😕

[nimmolo]

@mo-nathan Observations with lat/lng but no Locality are currently permitted, and I think what you discovered is not a validation failure, but that this change in the form streamlines creating such observations, which is not ideal (although it is temporary). Here's the logic in the controller:

# app/controllers/observations_controller/new_and_create.rb
  def determine_observation_location(observation)
    if Location.is_unknown?(observation.place_name) ||
       (observation.lat && observation.lng && observation.place_name.blank?)
      observation.location = Location.unknown
      observation.where = nil
    end
    observation
  end

[nimmolo]

~New consideration: if we want to order our Locality suggestions by increasing area, we should convert our location columns to geometric data so that MySQL can calculate the area for us, and orderby this value.~

---

I moved the above to discussion #2180. It seems outside the scope of the obs form refactor.

[mo-nathan]

In my view locality should not be required and in fact the current form claims it's required, but it's not and it defaults to "Earth". This seems reasonable to me (but we should probably removed the "(required)" test. Doing reverse geolocation would be awesome, but to me it is not required to get this phase of the work done.

[mo-nathan]

In my view it is bad that test.mushroomobserver.org is writing to the production database. Given that I will stop using it. We should point it at it's own database.

[JoeCohen]

I don't understand. In particular, is a separate server worth the trouble and expense if it's writing to a separate db?

On Sat, Jun 15, 2024 at 4:52 AM Nathan Wilson @.***> wrote:

In my view it is bad that test.mushroomobserver.org is writing to the production database. Given that I will stop using it. We should point it at it's own database.

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2155#issuecomment-2169388890, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALDFHTO7VR4U7WNYLAICTZHQTI3AVCNFSM6AAAAABIQFB42KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRZGM4DQOBZGA . You are receiving this because you were mentioned.Message ID: @.***>

[mo-nathan]

Hmm, I guess this is a matter of perspective. It's even possible that I suggested this configuration, but now that I'm experiencing it, it seems wrong to me. For me, a "test" server should be for testing so you can do crazy things with it to see if you can break something. On a daily basis, I don't have content that I can just upload to production for testing so I don't know how I would do through testing like Nimmo asked for. In my view, the right thing to do at the current stage of development is local testing. However, that didn't work for me, so I got lured into thinking that "test.mushroomobserver.org" was an actual test server. Maybe we should rename it to "preview" or just scrap the idea altogether. At work we have a set of "lower" environments for various types of testing. We start with local systems, then we have development (6), qa (2), loading testing (3), performance testing, staging, and user acceptance (4) servers that are all independent of actual production. While we certainly don't need to go to this extreme, I think what we're doing at work is more what the expected model is. The idea of running separate servers that point to the same database is inherently problematic since bugs in one of them may mess with the other one in pretty significant ways.

[JoeCohen]

@nimmo: That's the problematic obs.

@JoeCohen if this is the obs you tried to create with images, where your images were not attached to the obs, I think i found it. They sure aren't there, are they? 😕

[nimmolo]

@mo-nathan Did assets:clobber fix your dev environment?

[nimmolo]

@mo-nathan — I agree that "test" implies that you are encouraged to break stuff. But for now, what say we try to get through these obs-form PRs by continuing to test on "test" and just deleting our obs afterwards. That's what I've been doing — I've created untold "Corallorhiza" observations and deleted them.

It was a lot of work for me setting up the server on test again, because of my unfamiliarity setting up Linux servers — and just a couple months after giving the go-ahead to delete the previous test server. I do feel that we need one for the type of PRs we have in the pipeline currently.

[nimmolo]

@JoeCohen Are you able to replicate the disappearing images issue? Or suggest anything I can do to try to get the same result?

[JoeCohen]

@nimmolo: I cannot replicate that result. (I just tried several times.)

[nimmolo]

Revisions deployed to test. Please re-review

• Re-added the extra "Create" button. (Buttons will all likely be very different with the tabbed form in part 3 of these PRs)
• Renamed "Image Notes" - I agree this disambiguation is better.
• Also moved it down below camera info. I'm not crazy about this, though. Having it at the bottom of the form seems to discourage people from writing captions, which I'd rather people write. Anyway, we can all consider this placement.
• I'm not able to reproduce the "obs created with no images" bug. But please try to! EDIT Yes i can, but it's because the test server is slow. The images are there on reload.

[mo-nathan]

Did another create and got a broken image after just clicking create. However, the image showed up after a reload. Note that I was trying to get the observation to be created with as little info as possible. I just dropped the image and then scrolled down and clicked "Create".

[mo-nathan]

I also noticed that if you use the default location of "Earth" that clicking on the lat/long doesn't give you anything useful. I was hoping to see the lat/long on a map so I could remember exactly where that photo was from.

[nimmolo]

@mo-nathan - I believe the broken image is because the test server is slow.

I'm saving work on coordinating the Location, lat/lngs and the two(!!) maps til the next PR.

Note that there are two buttons to show objects on the map in this form — this is not new, although I find it confusing. There's a "Open interactive map" for the lat/lng, and there's a "Locate on map" for box locations. Using "Earth" as a location for the latter will disable the second button entirely, as I believe it should.

In the tabbed form of the mobile app, the lat/lng and date are in the first tab with the photo because they are derived from it — i.e., Lat/Lng is in a separate tab from the "Locality". I'm aiming to model our tabbed form after the mobile app, but let's discuss it.

So my current thinking is that the tabbed form may keep these two map UIs, so people can check their map lat/lng and their locality in each tab. I feel like the "locality" map should probably show the lat/lng point if there is one, though, so people can catch the correspondence to the locality — especially because we're going to be suggesting and in some cases auto-filling from an MO location, or auto-creating a Location from Google data.

[mo-nathan]

@nimmolo It makes me nervous that the test server being slow is causing a broken image. I'm thinking of the case where there are bunch of folks uploading images to MO at an event. I really want to test this against production, but it seems like something more graceful than a broken image would still make sense.

Regarding the map, I can't tell whether you think a lat/long listing with no location (== "Earth") is working as it should or if you're just saying that it's a separate issue (which I can agree with). The current behavior doesn't make sense to me. Why should it behave differently if I have "Earth" vs. "USA", but provide a precise lat/long (as it does at the moment).

[nimmolo]

@mo-nathan - I understand why that would make you nervous.

It occurred to me to check the behavior of the current main branch when it's running on the test server — do images appear with dead urls at first, and then they appear on reload? I just tried it, and that is the case. I've still got main running on test, so let me know if you get the same results. It may not be because the server is slow — it may be a config that I got wrong when setting up the server in the first place. I've sent a question to Jason about it.

Re maps: separate issue, yes. But maybe where the current UI is wrong is that once you've opened the map, there's no longer a button to mark your lat/lng on the map! Our map JS hides that button once the map is open. That does not seem right. You can only map the "Location" (box) you're picking once the map is open, you can't re-center it on your lat/lng (point).

[mo-nathan]

Re: the test server behavior, that makes some sense to me and I'll look at it this evening.

Re: the map behavior, I'm not sure I've explained the problem clearly enough. I'll create an issue this evening with screen shots. Hopefully I can find some time to fix it over the weekend. I've been working on understanding the gap between how field slips and project constraints should work and how they currently work on the site so I can get that sorted.

[nimmolo]

@mo-nathan - Jason figured out that it was indeed a config, and fixed it. We're still ironing out the configs to make sure this doesn't happen again, and to make the code portable to other hypothetical test servers. I'll commit whatever changes there are that should go into the repo.

I'm going to switch the test server back to this branch, and test it again. I'll also ask you and @JoeCohen to kick the tires one more time, and merge tonight if that's ok. I'm pretty far along on the second "location suggestions" PR.

[nimmolo]

@mo-nathan re: map behavior - yes, thank you. Rereading this thread i'm not sure I understood very well either, and in fact I know i described some things inaccurately a couple responses above. I've tried to correct those by editing the responses. But an issue with screenshots will make things clearer, i imagine.

[mo-nathan]

In my view this rubocop violation should be addressed: TODO found NEW

TODO: make this a component

Severity: MinorFound in app/views/controllers/observations/form/images/_carousel.erb by fixme

[mo-nathan]

If you want me to "kick the tires", I need to be able to run it on my local so I freely test it without impacting the production database.

[mo-nathan]

At the moment at least test.mushroomobserver.org doesn't have the new create observation UI.

[mo-nathan]

I looked at test.mushroomobserver.org because I noticed that locally this branch had the "Image notes" at the top of the panel. This still doesn't feel like the right thing to be expecting users to be doing first. I expect the majority of images to not have notes. I'm not saying we should do exactly the same thing but if you look at the workflow for iNat, they streamline it down to providing the name and inferring the date and location. The last thing they ask for are notes which are presumably notes for the observation not for the individual image.

I also think that "When it was taken" should be above the "Copyright holder" and the "License" selector should be right next to the "Copyright holder".

[mo-nathan]

Went back to try it locally and noticed that the text in the "Camera info" box looks off. Specifically it says "Use this infoInfo copied". I think it's intended to just say "Info copied" since the button is disabled, but I'm not 100% sure.

[mo-nathan]

When there is a second image uploaded the button becomes active, but it still says "Use this infoInfo copied".

[mo-nathan]

Given that the plan is for the location and date info to be inferred from the photos and we expect that to be used a lot, I find myself agreeing with iNat that the name selection should be higher on the page.

[nimmolo]

Responding on Slack!

[mo-nathan]

Documented the map issue here: https://github.com/MushroomObserver/mushroom-observer/issues/2187

[JoeCohen]

Problems with maps while creating Observations on the test server. (Sorry if I've overlooked something the conversations here, on email, or Slack.)

• Create Observation
• Drag photo to page Results: 👍 Date fills automatically 👍 lat/lng fills automatically
• Click Open Interactive Map button Result: 👎 Open Interactive Map button disappears, but there's no map.

[JoeCohen]

Suggestions re form:

1. More prominent instructions about Selecting/Dropping Files. Also maybe include something like "Step One".
2. Align geoloc help with Lat/Lon/Elevation (rather than Open Interactive Map).

[nimmolo]

@JoeCohen - I changed the button from "Open interactive map" to "Find point on map", and changed the behavior so that the button is not hidden when the map opens.

However! The Google Maps API will not serve the map to test.mushroomobserver.org because it's not a permitted domain for our API key. If you know how to permit it, that could be helpful for our testers too.

[nimmolo]

I think I just permitted test.mushroomobserver.org. Let's check if it works... Yes.

[JoeCohen]

You did permit it, and it fixes those issues.

[mo-nathan]

This is looking pretty good. What's left on your list? I spotted a bit of cleanup that you mentioned last night (like getting rid of the ID text box for locations and names and getting the "When" fields to be arranged horizontally rather than vertically when in mobile mode).

[mo-nathan]

I wonder if we should have a warning popup if you try to navigate away from the create observation page after you've uploaded an image. Given the complexity of that UI at this point I kind of lost track that it wasn't permanent when I went to change the theme to test that out. I don't think it would harm anything to just have a "Are you sure you don't want to create this observation?" modal popup if they have put in enough effort to have uploaded an image.

[mo-nathan]

Hmm, it's a bit more complicated than that. I see now that if you go to a read only page (like click on the "License" link) and then go back to the Create Observation page, then the state is saved. Even if you go to the get page for updating your preferences and immediately go back, the state is saved. However, if you change the state in preferences and do a save, then when you go back the state for the Create Observation page is lost. Oh and if you go to the "License" link and then click "Create Observation" rather than using the back button, then it initially shows the previous state which then vanishes and you have a blank form. This is definitely a tricky case. It make me think that these links should create new tabs, but that's a pretty big change and at least used to be frowned on by the HTML standards folks. However, it seems a lot more common these days. No idea if there's an easy way to do that generally from the Create Observation page and not from pages that don't have any relevant state.

[nimmolo]

Good points.

One thing: i haven't done anything about background image uploading to the server yet, so the state that you're noticing being saved is just browser memory: the local state of the page where the image has been loaded into the browser, not uploaded to MO. (It took me a few days to realize that these were two different steps, and that "loading into the browser memory" was even a thing. That step is what's necessary to show the image in the UI prior to uploading it.)

So although they may navigate and throw away their state, it's not costing our server anything.

[nimmolo]

We could execute some JS that adds a target="_blank" attribute to all links on the Create Obs page.

On the other hand, that adds complexity and i don't think users will make that mistake twice - they can always right-click a link to open in a new tab.

[nimmolo]

@JoeCohen - I moved the geo help up alongside lat/lng/alt as you suggested. I hope to redesign these help boxes in the next phase PR, so it may not be there long.