Field Slip Forms - Githubissues

mo-nathan commented 1 month ago

On this branch you can now add all the information requested by a field slip (except whether it has a photo or not) on the edit field slip page. The data will be stored and/or synched with the data in any associated observation. If no observation is currently associated with the field slip, then the field slip data will be added to the new observation. If a new observation is not created and you navigate away from the create observation page, then the new field slip data will be lost. Most of the field slip data gets stored as fields in the Notes section of the observation.

Currently there is separate date info associated with the field slip and the observation. Not sure that's right, but I don't know that it's a big deal and I can at least imagine situations where they could be different.

If a field slip id is added then the existing namings are checked and if that name has not been applied to any existing observation, then it gets applied with a default vote of "I'd Call It That". My goal with all of this is to make the "happy path" for recorders at events as straight forward as possible. There's still a bit of a rough edge in how someone at a computer should be scanning the QR codes. It's possible to use one of the online QR code readers, but it would be better if there was an option in the website, but I think that's work for another ticket.

In the process I also cleaned up the CRUD for field slips to bring them more in line with what we do elsewhere in the site.

Here are some specific scenarios I used while developing:

1) As a project admin, I would create a new field slip with a URL something like http://localhost:3000/qr/NFAL-00010. You’ll need to add yourself as an admin or login as me if you use the NFAL project. Fill out the form (play with the auto-completes including providing values that aren’t in MO) and click ‘Create New Observation’. Ensure that all the data gets saved and that you can edit the fields etc. Auto-complete values that match stuff in the database should get rendered as reasonable Textile.

2) As a non-admin create a new field slip with a similar URL and create a new observation, but don’t add much data (include no name or non-specific names). Then as an admin go to the same URL (should take you to the observation). Now click on the field slip on the right hand side. Fill in the data as though it were coming from a field slip, but now with a name from some mycologist. Ensure that the new data and name get applied correctly to the observation. Note that the naming will still come from you, but the Field Slip ID By field should show the mycologist’s name.

coveralls commented 4 weeks ago

coverage: 94.463% (+0.009%) from 94.454% when pulling 0ec9b7b24dc755ad462a90bbc014d3b853e2fc6f on njw-field-slip-forms into 255e5c395acb357a5b0f4763b41c548980fd39ee on main.

coveralls commented 4 weeks ago

coverage: 94.463% (+0.009%) from 94.454% when pulling 0ec9b7b24dc755ad462a90bbc014d3b853e2fc6f on njw-field-slip-forms into 255e5c395acb357a5b0f4763b41c548980fd39ee on main.

coveralls commented 4 weeks ago

coverage: 94.463% (+0.009%) from 94.454% when pulling 3c1501954b8e7f7ae3c0db97743b4571d34b711d on njw-field-slip-forms into 255e5c395acb357a5b0f4763b41c548980fd39ee on main.

coveralls commented 4 weeks ago

coverage: 94.463% (+0.009%) from 94.454% when pulling aafc150e5e12d6feb9bc59cef97ab93c80dab651 on njw-field-slip-forms into 255e5c395acb357a5b0f4763b41c548980fd39ee on main.

coveralls commented 3 weeks ago

coverage: 94.465% (+0.01%) from 94.454% when pulling 6d960871fe556e47ac627b57f5b2cc1195f6311f on njw-field-slip-forms into 444f99c5e476a2edf5e12b454b9b93a74c4373b9 on main.

coveralls commented 3 weeks ago

coverage: 94.428% (-0.03%) from 94.454% when pulling b1fe07bcdba611161b091f96dde2e0b431b0061c on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.428% (-0.03%) from 94.454% when pulling b1fe07bcdba611161b091f96dde2e0b431b0061c on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.428% (-0.03%) from 94.454% when pulling b1fe07bcdba611161b091f96dde2e0b431b0061c on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling a776cff668740d467c54b370d148b3714ce3dffc on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.486% (+0.03%) from 94.454% when pulling 9de2b31017796cb0431b9942ef38bf6e3e689102 on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.486% (+0.03%) from 94.454% when pulling 9de2b31017796cb0431b9942ef38bf6e3e689102 on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.48% (+0.03%) from 94.454% when pulling fdb91bd6f2492bda56776f432721b60611b490e4 on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

coveralls commented 3 weeks ago

coverage: 94.48% (+0.03%) from 94.454% when pulling fdb91bd6f2492bda56776f432721b60611b490e4 on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

mo-nathan commented 3 weeks ago

@nimmolo Ran into some surprising behavior that I think is related to some Turbo magic happening on the FieldSlip show page. Currently on the website if you go to the FieldSlip show page for a field slip, any associated observation becomes that user's last ObservationView. This was not the original intention of that page since if you then click 'Edit Field Slip', the 'Switch to Last Observation' option shows the same thing as the 'Save with Current Observation' option which makes the switch option confusing and useless. In digging into why, it looks like the FieldSlip#show page is now indirectly calling the Observation#show page through Turbo which is where update_view_stats is called which updates the ObservationView. I don't understand exactly why this is happening and the same behavior is not happening when you go to the FieldSlip#index page although it as well is display what are essentially the contents of the FieldSlip show page.

As a short term fix, I changed the "Last Observation" code in this PR to ensure that it doesn't return the exact same observation as the "Current Observation". While this is an improvement, it still doesn't really capture the intent of the feature. For example, if my intent is to connect Observation X with Field Slip Y and I go to Observation X, then go to the FieldSlip#index page and happen to click Field Slip Z, and then go back to the index and select Field Slip Y, then the "Last Observation" will be the observation associated with Field Slip Z, not Observation X.

TL;DR - I need a way for the FieldSlip show page to NOT have the side effect of calling update_view_stats due to indirectly calling the Observation show page.

coveralls commented 3 weeks ago

coverage: 94.48% (+0.03%) from 94.454% when pulling 6af3add6e81b05dc04ffdefd794d883d4f6dddf9 on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

JoeCohen commented 3 weeks ago

Observation missing from Project.

As Joe Cohen I created a Project (FSF Test) with no date constraints, Location: Earth, Field Slip prefix FSFTEST
As a non-admin (OMS) I created a field slip http://localhost:3000/qr/FSFTEST-00001
- a random observation appeared (http://localhost:3000/observations/535653) and I ended up using it for the Field Slip. I forget exactly what I clicked to make this happen.
- The Field Slip was created with that Observation. The Field Slip is included in the Field Slip index.
- Issues:
- The Observation is not associated with the Project

JoeCohen commented 3 weeks ago

Form -- Other Codes

The Other Codes field in the form could use a short explanation or example.

JoeCohen commented 3 weeks ago

When a non-admin creates a field slip, it would be nice if the Collector field were pre-populated with the current user.

JoeCohen commented 3 weeks ago

Some ideas for other PRs prior to NEMF:

An easy way in the UX for Project Admins to navigate to the Field Slip index. (A simple solution is adding something to the lh nav menu.)
FS Index: Each entry takes up too much space. It would be nice to have this as a table. And with very small thumbnails; something same size as a carousel image. (IMO the Images are way too big on this and many other pages. They push other content off the screen.)
FS Index: Should be searchable like other indexes, so that Recorder could display Field Slips for one Project.
FS Index: Should be sortable.
Clean up Users. Users with zero contribution who haven't accessed the site in a while (3 months) should be deleted as part of a chron job. That makes the User field type-ahead much more useful
Name typeahead should hide or distinguish deprecated Names (e.g. by lighter font or "(deprecated)".

JoeCohen commented 3 weeks ago

Field Slip suffixes

(I think we may have discussed this already, but my memory is fading.) Questions:

Should they be limited to numbers? If no, then limited to character set(s)? Ex.: Do you want to allow stuff like FSPREFIX-천안시 봉서산?
For numerical suffixes, do you want to allow both FSPREFIX-000001 and FSPREFIX-1? Seems confusing to me.

mo-nathan commented 3 weeks ago

Observation missing from Project.

As Joe Cohen I created a Project (FSF Test) with no date constraints, Location: Earth, Field Slip prefix FSFTEST

As a non-admin (OMS) I created a field slip http://localhost:3000/qr/FSFTEST-00001

a random observation appeared (http://localhost:3000/observations/535653) and I ended up using it for the Field Slip. I forget exactly what I clicked to make this happen.

The Field Slip was created with that Observation. The Field Slip is included in the Field Slip index.

Issues:

The Observation is not associated with the Project

Not able to reproduce this. At first I figured it would have something to do with using the last observation, but that got associated with the project in question just fine (but it didn't complain about the constraint violations of that observation so that's a bit of a bug).

mo-nathan commented 3 weeks ago

Form -- Other Codes

The Other Codes field in the form could use a short explanation or example.

Would something like " (e.g, iNat ID)" be enough?

mo-nathan commented 3 weeks ago

When a non-admin creates a field slip, it would be nice if the Collector field were pre-populated with the current user.

I like this idea. Why just non-admins? I would expect admins would often be adding their own observations. Recorders generally wouldn't be, but they'd have to fill in the value anyway.

mo-nathan commented 3 weeks ago

Some ideas for other PRs prior to NEMF:

An easy way in the UX for Project Admins to navigate to the Field Slip index. (A simple solution is adding something to the lh nav menu.)

I don't think this make sense for the lh nav. I have been wondering about adding a field slips tab to the project page that would filter by that project. I think that might address a couple of your thoughts.

FS Index: Each entry takes up too much space. It would be nice to have this as a table. And with very small thumbnails; something same size as a carousel image. (IMO the Images are way too big on this and many other pages. They push other content off the screen.)

I'll let you and @nimmolo arm wrestle on this one. I started with the thumbnail size ones and Nimmo objected. Just tell me what you guys decide. I have thought about switching the index to a table. Currently it's just using the show partial because that's what the rails generate scaffold does. What do you think of Thumbnail, Code, Collector, and ID? Should there be more columns? (I'm assuming it's scoped by project)

FS Index: Should be searchable like other indexes, so that Recorder could display Field Slips for one Project.

Does this get handled if the index is always scoped by the project? There's an interesting question about Field Slips that aren't associated with a Project. Not clear that those really need an index page. There could be an unscoped version that lists everything and adds a "Project" column which could be blank.

FS Index: Should be sortable.

Doesn't sound too hard. Is sorting by a single column sufficient?

Clean up Users. Users with zero contribution who haven't accessed the site in a while (3 months) should be deleted as part of a chron job. That makes the User field type-ahead much more useful

That seems like a good idea which anyone could implement (hint, hint :-))!

Name typeahead should hide or distinguish deprecated Names (e.g. by lighter font or "(deprecated)".

Can you give an example where this is really a problem? I would note that it is likely that recorders will be recording names that MO considers deprecated, but which the mycologists at the event still use.

mo-nathan commented 3 weeks ago

Field Slip suffixes

(I think we may have discussed this already, but my memory is fading.) Questions:

Should they be limited to numbers? If no, then limited to character set(s)? Ex.: Do you want to allow stuff like FSPREFIX-천안시 봉서산?

For numerical suffixes, do you want to allow both FSPREFIX-000001 and FSPREFIX-1? Seems confusing to me.

I don't see a strong reason to be strict about it. If you are using printed field slips then the suffix will always be 5 digit 0 padded numbers (unless they print values over 99999 which seems unlikely). If someone enters another value by hand, then the system accepts it. It seems like entering them by hand should be rare. All the field slips I've seen end in some sort of number, but that may not be universal. Also folks often seem to like to include a year and you could debate whether that is conceptually part of the prefix or the suffix.

coveralls commented 3 weeks ago

coverage: 94.48% (+0.03%) from 94.454% when pulling b415136f6faccca80452eaecdb352a0944573164 on njw-field-slip-forms into 8f8dec86b70d7f602e119c87b1bed26f49cd84f0 on main.

nimmolo commented 3 weeks ago

If you guys want smaller images everywhere, you're probably going to need a different front-end guy! Foregrounding the images and prying them out of all the verbose cruft and "Craig's List HTML" framing in the views was my #1 goal in getting involved with Mushroom Observer and trying to lead us on this long march to the fabled promised land of Bootstrap 4.

To me scrolling is a small price to pay for a much more attractive layout, and not having to use "theater mode" to get a good look at an image. (But there's room for improvement...)

As to why i have the images bigger on create obs: if a user didn't notice a photo is blurry or otherwise not ideal before uploading, now is the time to catch it, as there is no "theater mode" on create obs. The image has to be big enough to evaluate quality-wise. Overall, the images section takes up less vertical room now than before, if there are more than two images on the obs. Tabbing/paging the form should help further - my goal is a form like the mobile app.

However, big images are of course not necessary in cases where the image is not what someone is looking for - like this field slip view.

One other thing i do care about moving towards, front-end-wise, though, is reusable components. They not only give a rhythm to the design and DRY up the code, but clue the user in to the type of info they contain. When someone is used to them, they can scan the page with much less eyestrain.

Sounds like what we need is a more compact "table-row"-like component for this and other admin-y views, so i say let's design and build one, so we can use it consistently. I had suggested the matrix-box here but i was only picturing a few obs. How many obs are there likely to be on this view? Dozens? Hundreds?

nimmolo commented 3 weeks ago

I do like the idea of the name autocompleter differentiating between MO-deprecated and approved names, without forcing the latter. I feel like this would make the naming form less of a gotcha for newbies.

Refactoring the autocompleters to return better data - JSON objects instead of strings, including the record IDs - is on my shortlist.

mo-nathan commented 3 weeks ago

In my view the big images in most places make sense. I believe the issue is specifically around the FieldSlip index page. I'm fine with how things are on the FieldSlip show page, but I would also be fine with a smaller size more like what's on the observation index page. If the FieldSlip index page is going to get used regularly then I agree with Joe that we should have a more compact table-ish layout. I expect there will ultimately be 10s of 1000s of FieldSlips if they are widely adopted. A single NAMA foray often has in excess of 1000 collections. For NEMF we're planning on printing 4,800 field slips and I expect we'll actually use over 1000 in October. As a rough estimate, there are already more than 20,000 project observations with the largest project having over 2000 observation. Given that, even with filtering by project, the page is going to have to be paginated to load reasonably quickly. I expect we'll want each page to have something like 50 rows. Having some sort of thumbnail image of any associated observation make sense to me. Other than that, I don't have a lot of opinion about exactly how it looks.

JoeCohen commented 3 weeks ago

I'll see if I can replicate this, paying more attention to what I'm doing.

Observation missing from Project.

As Joe Cohen I created a Project (FSF Test) with no date constraints, Location: Earth, Field Slip prefix FSFTEST

As a non-admin (OMS) I created a field slip http://localhost:3000/qr/FSFTEST-00001

a random observation appeared (http://localhost:3000/observations/535653) and I ended up using it for the Field Slip. I forget exactly what I clicked to make this happen.

The Field Slip was created with that Observation. The Field Slip is included in the Field Slip index.

Issues:

The Observation is not associated with the Project

Not able to reproduce this. At first I figured it would have something to do with using the last observation, but that got associated with the project in question just fine (but it didn't complain about the constraint violations of that observation so that's a bit of a bug).

JoeCohen commented 3 weeks ago

Yes; " (e.g, iNat ID)" would be great.

Form -- Other Codes

The Other Codes field in the form could use a short explanation or example.

Would something like " (e.g, iNat ID)" be enough?

JoeCohen commented 3 weeks ago

Makes sense.

Field Slip suffixes

(I think we may have discussed this already, but my memory is fading.) Questions:

Should they be limited to numbers? If no, then limited to character set(s)? Ex.: Do you want to allow stuff like FSPREFIX-천안시 봉서산?

For numerical suffixes, do you want to allow both FSPREFIX-000001 and FSPREFIX-1? Seems confusing to me.

I don't see a strong reason to be strict about it. If you are using printed field slips then the suffix will always be 5 digit 0 padded numbers (unless they print values over 99999 which seems unlikely). If someone enters another value by hand, then the system accepts it. It seems like entering them by hand should be rare. All the field slips I've seen end in some sort of number, but that may not be universal. Also folks often seem to like to include a year and you could debate whether that is conceptually part of the prefix or the suffix.

mo-nathan commented 3 weeks ago

Added example text to "Other Codes" and made the default value for "Collector" smarter. Realized that if a field slip was created by a user and doesn't have a "Collector", then they are probably the better default than the current user. Now that there is a default, this should be a rare edge case, but seems worth getting right given that there are existing field slips out there.

I moved the discussion of improvements to the FieldSlip index page to it's own discussion, #2179. Other than @JoeCohen trying to replicate the Case of the Missing Project, I think this is ready to go.

coveralls commented 3 weeks ago

coverage: 94.479% (+0.03%) from 94.454% when pulling 4aa6318f545ac20f7de30368fc88bd2d10143808 on njw-field-slip-forms into 38b163f25e902ab821d7e49b2e20c148720ec850 on main.

coveralls commented 3 weeks ago

coverage: 94.479% (+0.03%) from 94.454% when pulling 4aa6318f545ac20f7de30368fc88bd2d10143808 on njw-field-slip-forms into 38b163f25e902ab821d7e49b2e20c148720ec850 on main.

coveralls commented 3 weeks ago

coverage: 94.479% (+0.03%) from 94.454% when pulling 4aa6318f545ac20f7de30368fc88bd2d10143808 on njw-field-slip-forms into 38b163f25e902ab821d7e49b2e20c148720ec850 on main.

JoeCohen commented 3 weeks ago

I can reliably replicate the Case of the Missing Project.

As user 1(Joe Cohen), in browser 1 (I used Chrome)
- I had created open project FSF Test, with Field Slip prefix FSFTEST, Location: USA, no date constraints
In another browser (I used Safari)
- login as another user (OMS)
- Paste http://localhost:3000/qr/FSFTEST-00010 into the address bar
- Select Use Last Observation Result:
- Creates Field Slip
- Associates last Observation with Field Slip
- Says that the Observation is in project FSF Test
Back in Chrome as User 1
- Refresh show project window. Result: # of Observations has not changed
- Click Observations Result: Does not list the Obs which is associated with FSFTEST-00010
Back in Chrome as User 2 (on the show field slip page)
- Click the project name (FSF Test) Result: Same as for the admin. Does not includ the Obs which is associated with FSFTEST-00010

I will poke around and see if I can make any progress.

JoeCohen commented 3 weeks ago

Adding a FieldSlip to a Project does not automatically add (a) the Obs associated with the FieldSlip to (b) the Project. Is that intentional? (I didn't expect it.) If intentional, please ignore me. If unintentional: Suggestions:

In FieldSlipsControllerTest"should create field_slip with last viewed obs" (and maybe in other methods), assert that the Observation is in the project
In FieldSlipsController#create, add the Obs to the project unless params[:commit] == :field_slip_create_obs.t Something like -- after update_observations_fields
```
        proj = Project.find(params.dig(:field_slip, :project_ed))
        proj.add_observation(@field_slip.observation)
```

JoeCohen commented 3 weeks ago

And the same issue exists when creating a Field Slip and creating an Observation: The Field Slip gets added to the Project, but the Observation does not.

mo-nathan commented 3 weeks ago

Joe and I talked it over and the root issue is that the project he was creating wasn't "open" so the user and the observation could not be added. In talking it over we decided that given that situation the field slip should also not be added. I also think that given the user a warning that this has happened would also make sense. Something like "The project associate with this field slip, , is currently not allowing users to add themselves. Please contact a project admin if you want to join the pojrect.". It should probably also reject an observation/field slip if the observation violates the project constraints. This also implies that the field slip should be removed from the project if a newly created observation turns out to violate the project constraints (and it doesn't get overridden).

mo-nathan commented 3 weeks ago

[x] Don't add field slips to closed projects that the user is not a member of.
[x] Disable changing a field slip code on the edit page or from a QR code redirect.
[x] "Record Field Slip" (FieldSlip new) should ask for the code first and then act like the QR code redirect.
[ ] Check project constraints for observations and provide more errors/warnings. In general, only admins should be able to violate project constraints.

coveralls commented 3 weeks ago

coverage: 94.479% (+0.03%) from 94.454% when pulling 6ec9c53f558cdb8893cf523e7a5a366244cbbc93 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 3 weeks ago

coverage: 94.479% (+0.03%) from 94.454% when pulling 6ec9c53f558cdb8893cf523e7a5a366244cbbc93 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 3 weeks ago

coverage: 94.479% (+0.03%) from 94.454% when pulling 6ec9c53f558cdb8893cf523e7a5a366244cbbc93 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 2 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling c3ae27a40b63a30cbf53e16e9dfabadd0b6024e5 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 2 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling c3ae27a40b63a30cbf53e16e9dfabadd0b6024e5 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 2 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling c3ae27a40b63a30cbf53e16e9dfabadd0b6024e5 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 2 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling dd19d3da33e158c368ded73171776250dbc61699 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 2 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling dd19d3da33e158c368ded73171776250dbc61699 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

coveralls commented 2 weeks ago

coverage: 94.482% (+0.03%) from 94.454% when pulling dd19d3da33e158c368ded73171776250dbc61699 on njw-field-slip-forms into b42b7a7457856d910019f10b17d22298abed812d on main.

MushroomObserver / mushroom-observer