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

3423 Recording Panorama Dates and Histories from GSV API #3527

Merged
merged 19 commits into from
May 7, 2024

Conversation

srihariKrishnaswamy
Copy link
Collaborator

Resolves #3423

In this PR, I implemented functionality to record all the panorama histories of each panorama that all users visit, and store them in a new table in the database in case we want to use that data later. I did this by first modifying the frontend to collect and keep track of the visited pano ids & histories given by the GSV API for each explore and validate mission that the user does. I then made a table in the database and wrote some backend code inside the existing explore and validate data submission routes to get that data from the frontend and store it in the new table.

The schema for the table is the following:
panorama_id (primary key)
visited_timestamp (timestamp)
pano_month (int)
pano_year (int)
location_current_pano_id (foreign key pointing to an existing panorama_id in the same table)

The idea here is that if we want to get all the previous panos at the same location as some pano, we can look for all panos with a location_current_pano_id matching that panorama's location_current_pano_id.

Testing instructions
  1. Delete everything from the table using the deleteAll() method.
  2. Write a print statement in the backend route (validation or audit route) that prints out the method call of selectAllPanoramaHistories(). This will print out all the rows in the table.
  3. Step through a label or validate mission and observe how the table changes
  4. If you'd like to test modifying a row, what I did was write a quick method in the PanoramaHistoryTable.scala that inserts a row with a primary key that's already in the table, and different data for the other columns. Re-print out the entire table and observe that that row is different.
Things to check before submitting the PR
  • I've written a descriptive PR title.
  • I've added/updated comments for large or confusing blocks of code.
  • I've tested on mobile (only needed for validation page).

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I didn't have time to do like a FULL review and test it, but I took a quick look and added some comments!

conf/evolutions/default/220.sql Outdated Show resolved Hide resolved
conf/evolutions/default/221.sql Outdated Show resolved Hide resolved
app/controllers/TaskController.scala Outdated Show resolved Hide resolved
app/formats/json/PanoramaHistoryFormats.scala Outdated Show resolved Hide resolved
conf/evolutions/default/221.sql Outdated Show resolved Hide resolved
conf/evolutions/default/221.sql Outdated Show resolved Hide resolved
app/formats/json/TaskSubmissionFormats.scala Outdated Show resolved Hide resolved
app/models/gsv/PanoramaHistoryTable.scala Outdated Show resolved Hide resolved
app/formats/json/PanoramaHistoryFormats.scala Outdated Show resolved Hide resolved
@srihariKrishnaswamy
Copy link
Collaborator Author

Adding a comment for myself: need to fix what happens in the case where i save null as the visited timestamp in the case that a row for that panorama already exists by adding logic to check if it's already in the table.

@srihariKrishnaswamy
Copy link
Collaborator Author

Alright, edits up!

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay review done! A bit more to restructure, but it's looking better!

conf/evolutions/default/220.sql Outdated Show resolved Hide resolved
conf/evolutions/default/221.sql Outdated Show resolved Hide resolved
app/models/gsv/PanoHistoryTable.scala Outdated Show resolved Hide resolved
app/formats/json/TaskSubmissionFormats.scala Outdated Show resolved Hide resolved
app/controllers/TaskController.scala Outdated Show resolved Hide resolved
conf/evolutions/default/221.sql Outdated Show resolved Hide resolved
app/models/gsv/PanoHistoryTable.scala Outdated Show resolved Hide resolved
conf/evolutions/default/221.sql Outdated Show resolved Hide resolved
public/javascripts/SVLabel/src/SVLabel/data/Form.js Outdated Show resolved Hide resolved
@srihariKrishnaswamy
Copy link
Collaborator Author

srihariKrishnaswamy commented Apr 27, 2024

Alright, with the assumption that the differences in nullability in both validation and label are ok and that I can use the last_viewed field as opposed to creating a new pano_history_saved field, I went ahead and made all the changes. Once you okay them, I'll get rid of some code that's not necessary barring those conditions and push up a new commit!

@srihariKrishnaswamy
Copy link
Collaborator Author

Ok, fixed that error I was talking about & the changes should be up!

@srihariKrishnaswamy
Copy link
Collaborator Author

Ok, the way this works now is we just enter new entries in the PanoHistory table every time a panorama is visited on the frontend (at most 1x per mission), like previously. The PanoHistory table has an f-key to the gsv_data table (for location current pano Ids). Every pano in the history (including the current one at the location) is stored inside the PanoHistory table, since I figured we were kinda just storing stuff as opposed to being too worried about formatting.

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some final cleanup of things, simplifications, etc. Merging now!

@misaugstad misaugstad merged commit 06acade into develop May 7, 2024
@misaugstad misaugstad deleted the 3423-record-dates-and-pano-ids branch May 7, 2024 20:21
@misaugstad misaugstad mentioned this pull request May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Start recording dates and pano IDs for older panos when users visit them
2 participants