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.

Sign up for GitHub

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

Jump to bottom

Add location grouping views #1334

Merged
merged 3 commits into from
Feb 24, 2023
Merged

Add location grouping views #1334

merged 3 commits into from
Feb 24, 2023

Conversation

artonge
Copy link
Collaborator

@artonge artonge commented Oct 13, 2022 •
edited
Loading

Needs: #1335

Screenshot 2022-10-26 at 15-22-29 Photos - Nextcloud

Screenshot 2022-10-26 at 15-30-48 Photos - Nextcloud

@artonge artonge added the 2. developing Work in progress label Oct 13, 2022
@artonge artonge added this to the Nextcloud 26 milestone Oct 13, 2022
@artonge artonge self-assigned this Oct 13, 2022
@artonge
Copy link
Collaborator Author

artonge commented Oct 13, 2022

/backport to stable25

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Oct 13, 2022
@artonge artonge added the enhancement New feature or request label Oct 13, 2022
@artonge artonge changed the base branch from master to artonge/feat/reverse_geocoding_cli October 13, 2022 08:55
@artonge artonge force-pushed the artonge/feat/reverse_geocoding_cli branch from 6bb4679 to 01ee346 Compare October 13, 2022 08:58
@artonge artonge changed the base branch from artonge/feat/reverse_geocoding_cli to artonge/feat/location_dav_endpoint October 13, 2022 08:59
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from fd0b76d to 907f072 Compare October 13, 2022 09:04
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch 2 times, most recently from dceccde to 218b33d Compare October 25, 2022 16:27
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from 907f072 to 644d044 Compare October 25, 2022 16:29
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch 4 times, most recently from a7cbc75 to 351a796 Compare October 26, 2022 09:25
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch 3 times, most recently from e7c44ab to f78095d Compare October 26, 2022 13:15
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch from 351a796 to 619e08d Compare October 26, 2022 13:16
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from f78095d to 41d33a6 Compare October 26, 2022 13:24
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch from 619e08d to d5df9dc Compare October 26, 2022 13:26
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from 41d33a6 to 08966d2 Compare October 26, 2022 13:28
@artonge
Copy link
Collaborator Author

artonge commented Oct 26, 2022

@jancborchardt for early feedback :)

@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch from d5df9dc to d652092 Compare October 26, 2022 13:35
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from 08966d2 to c18d8bb Compare October 27, 2022 12:02
@artonge
Copy link
Collaborator Author

artonge commented Nov 2, 2022

Or @nimishavijay

@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from c18d8bb to d3bebfe Compare November 2, 2022 12:14
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch from d652092 to 6d2efde Compare November 2, 2022 12:16
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch from f73c4af to a137a6e Compare February 15, 2023 15:18
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from de84539 to 40cfa80 Compare February 15, 2023 15:18
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch 4 times, most recently from 43a965d to 77fda9a Compare February 22, 2023 16:53
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch 3 times, most recently from d25af00 to 1988d94 Compare February 23, 2023 11:54
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
Merged
@artonge artonge force-pushed the artonge/feat/location_grouping_views branch from 1988d94 to ec6219b Compare February 23, 2023 17:32
@artonge artonge force-pushed the artonge/feat/location_dav_endpoint branch from 0749728 to 5bd766c Compare February 23, 2023 17:32
artonge
artonge commented Feb 23, 2023
View reviewed changes
appinfo/routes.php Outdated Show resolved Hide resolved
src/router/index.js Show resolved Hide resolved
Base automatically changed from artonge/feat/location_dav_endpoint to master February 24, 2023 07:26
skjnldsv
skjnldsv reviewed Feb 24, 2023
View reviewed changes
cypress/e2e/albums.cy.js

cy.reload()

cy.contains('New location')
cy.contains('New place')
Copy link
Member

Choose a reason for hiding this comment

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

It seems a bit too broad. We can merge but I would make sure to check against the specific section that contains this string, not the entire page

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was indeed too broad :)
#1657

@artonge @skjnldsv
Add location grouping views
0234a47 
Signed-off-by: Louis Chemineau <louis@chmn.me>
@skjnldsv
Copy link
Member

skjnldsv commented Feb 24, 2023 •
edited
Loading

Followups:

  • The location column of the photos_albums table is still present, since this is now a dav property, they should be migrated
    image
    image
  • Can you clarify if the albums should be displayed in the Places section? Because I find the two confusing. Files can have a location metadata, but Albums can have a location property. Only the files metadata seems to be used.
  • You manually have to run photos:map-media-to-place for the files location to be updated, right? Is there no on-upload or on-update file processing?

@artonge @skjnldsv
Rename locations to places
eac254e 
Signed-off-by: Louis Chemineau <louis@chmn.me>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Feb 24, 2023
@skjnldsv skjnldsv force-pushed the artonge/feat/location_grouping_views branch from 049950b to eac254e Compare February 24, 2023 07:55
@skjnldsv
Fix cypress
77df935 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv merged commit 541a388 into master Feb 24, 2023
@skjnldsv skjnldsv deleted the artonge/feat/location_grouping_views branch February 24, 2023 08:52
@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

@artonge
Copy link
Collaborator Author

artonge commented Feb 27, 2023 

* [ ]  The location column of the photos_albums table is still present, since this is now a dav property, they should be migrated

Not sure. See answer for the second followup.

* [ ]  Can you clarify if the albums should be displayed in the `Places` section? Because I find the two confusing. Files can have a location metadata, but Albums can have a location property. Only the files metadata  seems to be used.

'Place' for a file !== 'Location' for an album

Each file can have a place from its GPS coordinate.
Each album can have a location set by the user.

* [ ]  You manually have to run `photos:map-media-to-place` for the files location to be updated, right? Is there no on-upload or on-update file processing?

A background job was added to do the mapping in the background after the migration to the new version. The command is here to make it faster or to remap a user's files.

@artonge
Copy link
Collaborator Author

artonge commented Feb 27, 2023 •
edited
Loading

Thanks for pushing it to the finish line and merging :)

@artonge artonge mentioned this pull request Feb 28, 2023
Merged
@joshtrichards joshtrichards mentioned this pull request Jul 16, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

Assignees

@artonge artonge

Labels
4. to release Ready to be released and/or waiting for tests to finish backport-request Pending backport by the backport-bot enhancement New feature or request
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
3 participants
@artonge @nimishavijay @skjnldsv