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

iNat Import #2150

Merged
merged 481 commits into from
Sep 22, 2024
Merged

iNat Import #2150

merged 481 commits into from
Sep 22, 2024

Conversation

JoeCohen
Copy link
Member

@JoeCohen JoeCohen commented May 19, 2024

PR aims to implement the minimum acceptable functionality for NEMF for importing iNat observations

  • Allows an MO user to import a list of that User's iNat observation(s) to MO via the iNatImport form.
  • Implements the Basic Inat Import milestone

Note

  • It uses an inefficient way of giving users feedback about jobs. I didn't know how to do the js-Turbo 🪄 voodoo. I think that's enough for NEMF because I removed the UI for importing all a user's observations.
  • It doesn't rate limit. I think that's OK for NEMF for the the same reason.

Questions for reviewers

  • Is more functionality needed?
  • Appearance
  • General organization
  • Better names for classes, methods, variables, ex: ImportedInatObs
  • Any ideas about shortening/dividing long classes/methods
  • Any ideas about fixing ABC violations
  • Any other criticism is welcome.

Manual Test Protocol

  • Create a sacrificial iNat observation, making sure it has lat/lng. I suggest also selecting Obscured Geoprivacy.
  • In MO: Make sure SolidQueue is running, Create Observation, Import from iNat http://localhost:3000/observations/inat_imports/new
  • Fill in the fields, using the iNat observation # of the sacrificial iNat obs, and Submit
    Result: redirects to Observation Index (to be change to Inat Import Job Tracker)
  • Wait 10 seconds, then refresh screen
    Expected result: Index includes a new Observation
  • Goto the new Observation. and review it. The Location should make sense. If the iNat obs had Obscured Geoprivacy, then MO coordinates should be hidden from the public.
  • Goto/refresh the sacrificial iNat Obs
    Expected result:
    Description includes imported to MO and date of the import;
    Observation Fields include Mushroom Observer URL linking to the newly created Local observation.
    (In the production environment, it links to the live db.)
  • Delete the sacrificial iNat Obs

@JoeCohen JoeCohen linked an issue May 19, 2024 that may be closed by this pull request
14 tasks
@coveralls

This comment was marked as outdated.

@JoeCohen JoeCohen self-assigned this Jun 2, 2024
@nimmolo

This comment was marked as resolved.

@JoeCohen

This comment was marked as outdated.

@JoeCohen JoeCohen added the iNat interaction with iNat label Jul 18, 2024
- Gets multiple obss per request. For each request, gets pages; for each page gets obss
- Gets commoa separated list of obss
- Updates search obss mocks and InatObsTest accordingly
- Fixes controller test failuers
- Fixes error in prior commit
- Removes restriction in controller
- Removes now-unneeded tests, translation string
- Includes test which exposed the typo
- Deletes obsolete `import_one_observation` method that made 1 iNatAPI request per observation
- Prevents MO user from importing other people's iNat obss
(Currently doesn't completely prevent it; there's a workaround. But it can be prevented after authentication is functional.)
- Add checkbox to form
- Add appropriate translation strings
- Require user to (a) check import all, and/or (b) provide a list of obss
- Assert specific flash text (vs just any old warning)
- Includes username in params
- Removes unnecessary code. (If there's no consent, it never calls the iNat API.)
- Trvial refacor of `create`
- Prominently display refresh button if job is incomplete
- Alway display Your Observations button if job is done (including when there are errors).
@JoeCohen JoeCohen mentioned this pull request Sep 17, 2024
20 tasks
@JoeCohen
Copy link
Member Author

@mo-nathan Here's what I did to the tracker page. I don't have strong feelings about this, so am happy to make any other changes you'd like. Some options:

  • Change the button name to "Refresh to monitor the status", "Click to update status" or anything else.
  • A flash like "Import started. Click the Update button to update status"when the user (first) lands on the page.
  • Additional text like what you proposed.
Screenshot 2024-09-17 at 2 37 22 PM

Since the Turbo/JS magic isn't updating the page, I think the page should say something about manually refreshing the page. Maybe after the "Status" if the status isn't "Done", it could say "Refresh to monitor the status" or something along those lines.

- Update it if the job is successful enough to believe that inat_username likely belongs to the current MO user.
- Prevents user from importing someone else's iNat observation
- Prevents user from "stealing" someone else's iNat username
- Prevents user from accidentally changing his own`User.inat_username`
- Fixes finding mismatch
- Displays plain-language message if a mismatch
- Prevent stub persistence in `inat_import_job_test`
- Fixes bugs in test and un-Skips it.
- Adds comment explaining setup and teardown
Seems unnecessary. Also hoping it fixes the stub-related CI failures.
@JoeCohen
Copy link
Member Author

Saving and auto-filling the iNat username is implemented as of Commiit #276338c

Hoping this time it fixes CI failures.
Also conforms stub url to other calls to iNat API
- Extracts method
- Adds 1 sec delay between 2 iNat API requests that are very close to each other in order to comply with iNat Policy.
https://www.inaturalist.org/pages/developers#terms-of-use
- Tries to fix Codeclimate Line length offense
- Replaces a string with many interpolated values with iterating over a hash.
@JoeCohen JoeCohen merged commit df13cba into main Sep 22, 2024
7 of 8 checks passed
JoeCohen added a commit that referenced this pull request Sep 24, 2024
- Generates methods by iterating over an array, instead of many longhand defintions.
- Responds to #2150 (comment)
JoeCohen added a commit that referenced this pull request Oct 1, 2024
- Fixes InatImportsController#create ABC offense
See #2150 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iNat interaction with iNat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iNat import
4 participants