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

Venue post_type does not take care about the WordPress Geodata Standard. #560

Open
Tracked by #557
mauteri opened this issue Feb 23, 2024 · 12 comments
Open
Tracked by #557
Assignees

Comments

@mauteri
Copy link
Contributor

mauteri commented Feb 23, 2024

https://codex.wordpress.org/Geodata

@carstingaxion
Copy link
Collaborator

Thanks for keeping this @mauteri.

I wanted to give some more information on my initial attempt and why the WordPress Geodata Standard could be from interest for GatherPress' gp_venue post type and maybe even for the gp_event post type as well.

The codex says:

WordPress does not include any geographic functionality or functions to deal with Geodata itself [...].
But there is a standard for how to store geodata, so that plugins can interoperate.

The W3C Geolocation API([1]), introduced in 2014, offers the additional geolocation properties of altitude, accuracy, altitudeAccuracy, speed, and heading, which if adopted by any plugin to ensure compatibility with this, should be stored with the geo_ prefix.

If GatherPress wanted to follow this standard, it must make sure to save geo-related data within specially named custom-fields aka post_meta following this given format:

  • geo_latitude REQUIRED
    (float) decimal degrees -90 to 90 (negative values are in the southern hemisphere).
  • geo_longitude REQUIRED
    (float) decimal degrees -180 to 180 (negative values are in the western hemisphere).
  • geo_public OPTIONAL
    (int) is the geodata public (1) or private (0)? If value is missing or not set to 0, assume public.
  • geo_address OPTIONAL
    (string) freeform textual description of coordinates.

Because of the required fields for latitude and longitude, it's not possible to provide the needed data with the current version of GatherPress, the required data is missing.

So we would at least need to:

  • Request and save latitude and longitude data from the geo-data API (whichever we use)
  • Bind geo_publicdirectly to post_status
  • and geo_address is already available

While working on #638 I found out and tested for the first time, if and that it's possible to work with non-existent post_meta. This allows GatherPress to keep its current data structure, while still aligning with the WordPress Geodata Standard.

Example pseudo-post_meta for geo_address

  1. Register the post_meta (like normal)

    'geo_address' => array(
    	'auth_callback'     => function () {
    		return current_user_can( 'edit_posts' );
    	},
    	'sanitize_callback' => 'sanitize_text_field',
    	'show_in_rest'      => true,
    	'single'            => true,
    	'type'              => 'string',
    ),
  2. Load a filter to hook into the post_meta retrival

    \add_filter(
    	'get_post_metadata',
    	array( $this, 'get_pseudo_post_meta' ),
    	10,
    	3
    );
  3. Make sure data is delivered

    function get_pseudo_post_meta( $metadata, $object_id, $meta_key ): mixed {
    	if ( ! in_array( $meta_key, ['geo_latitude','geo_longitude','geo_address'] ) ) {
    		return $metadata;
    	}
    	$venue_information = (array) json_decode( get_post_meta( $object_id, 'venue_information', true ) );
    	$output = [];
    
    	switch ($meta_key) {
    		case 'geo_latitude':
    			$output[] = '48.856613'; // Paris
    			break;
    		
    		case 'geo_longitude':
    			$output[] = '2.352222'; // Paris
    			break;
    		
    		case 'geo_address':
    			$output[] = ( isset( $venue_information['fullAddress'] ) ) ? $venue_information['fullAddress'] : '';
    			break;
    		
    		default:
    			break;
    	}
    
    	return $output;
    }

This worked very well and I was able to see my pseudo-post_meta in action, using the Simple Location – WordPress plugin, which is following the WordPress Geodata Standard.

So, why not start saving latitude and longitude and aligning with the WordPress Geodata Standard?!!

@shawfactor
Copy link

I don’t think filtering the metadata is the right approach at all. Surely the plugin should just save the metadata using the right fields as described by the standard

More broadly saving all the venue information in one field in post meta is far from ideal anyway. In an ideal world every separate bit of information would have its own meta_key.

@carstingaxion
Copy link
Collaborator

Thank you for sharing your opinion @shawfactor

Surely the plugin should just save the metadata using the right fields as described by the standard

Yes.

More broadly saving all the venue information in one field in post meta is far from ideal anyway.

Yes. Yes.

In an ideal world every separate bit of information would have its own meta_key.

Yes. Yes. Yes.

I don’t think filtering the metadata is the right approach at all.

Yes. Yes. Yes. Yes.

But...

I was a little bit afraid of touching the principal data-architecture, because I only know about GatherPress since a short time.

I haven't measured it, but thought about this in particular for a while now: I think there shouldn't be any negative performance-impacts by providing non-existent meta-data based on existing meta-data.

And because we are going to render this data using the Block Bindings API, we want to create a render callback either.

@mauteri
Copy link
Contributor Author

mauteri commented Jun 7, 2024

@stephenerdelyi @jmarx this is the data ticket for venues

@mauteri mauteri added this to the 0.30.0 milestone Jun 14, 2024
@MervinHernandez
Copy link
Collaborator

✅ Viewed

@MervinHernandez
Copy link
Collaborator

✅ Reviewed June 22 = Confirming this has been discussed to be merged into the OpenStreetMaps work.

@mauteri mauteri moved this from Next Release to In Progress in GatherPress Project Jul 20, 2024
@mauteri mauteri modified the milestones: 0.30.0, 0.31.0 Jul 22, 2024
@mauteri mauteri moved this from In Progress to Following Release in GatherPress Project Jul 22, 2024
@mauteri
Copy link
Contributor Author

mauteri commented Jul 22, 2024

@MervinHernandez this ticket opened up a bit of a can of worms for me. The approach above was a bit buggy because of the nature of the block editor I believe. I've punted this ticket to 0.31.0 because I think the data just needs to be overhauled as discussed above. I think we might even want to break into multiple Venue blocks so Address, Phone Number, and Website aren't tied to tightly together, but I want to chat about that a bit on Friday.

@carstingaxion
Copy link
Collaborator

carstingaxion commented Jul 22, 2024

I think we might even want to break into multiple Venue blocks so Address, Phone Number, and Website aren't tied to tightly together,

Your statement surprises me a bit, because this separation of concerns is exactly what I described in #626 (in general), in #629 (especially for the new venue block), and what I'm aiming for.

Given that I have still not described the details of #638, I can understand your confusion.

Overall the refactored, venue-related blocks will be IMHO :

When using those blocks from within an event- or other post type, you'll have to wrap them in a new venue-details block (#629), which is already working and can be tested in the GatherPress block playground.

@mauteri
Copy link
Contributor Author

mauteri commented Jul 22, 2024

I think we might even want to break into multiple Venue blocks so Address, Phone Number, and Website aren't tied to tightly together,

Your statement surprises me a bit, because this separation of concerns is exactly what I described in #626 (in general), in #629 (especially for the new venue block), and what I'm aiming for.

Given that I have still not described the details of #638, I can understand your confusion.

Overall the refactored, venue-related blocks will be IMHO :

When using those blocks from within an event- or other post type, you'll have to wrap them in a new venue-details block (#629), which is already working and can be tested in the GatherPress block playground.

Yes, this is where I ended up too, though I could see Map / Address as the same block and they are tightly coupled. Website and Phone can be their own blocks.

Don't be surprised by me :-) A bunch of these were written years ago and I'm starting to look at things through a fresh, new lens. How we saw the project 2+ years ago and how we see it now has changed, and so has a lot of the components of it.

@mauteri mauteri moved this from Following Release to Next Release in GatherPress Project Aug 16, 2024
@carstingaxion
Copy link
Collaborator

carstingaxion commented Aug 21, 2024

Let's make sure, that the new metas have revisions enabled. IMO this makes sense for all (the formerly mentioned) meta, which just needs to be registered separately.

https://make.wordpress.org/core/2023/10/24/framework-for-storing-revisions-of-post-meta-in-6-4/

@MervinHernandez
Copy link
Collaborator

MervinHernandez commented Oct 24, 2024

QUEUED to Discuss - October 25

  • Jeff M has work towards this
  • Consider merging with other lat-long related work

@MervinHernandez
Copy link
Collaborator

MervinHernandez commented Oct 25, 2024

In works - not ready for testing just yet.


Chat
1 Breakup blocks
2 Tie to individual meta
3 Add button to reset
4 Run script to cleanup output

@MervinHernandez MervinHernandez modified the milestones: 0.31.0, 0.32.0 Oct 25, 2024
@MervinHernandez MervinHernandez moved this from In Progress to Icebox in GatherPress Project Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Icebox
Development

No branches or pull requests

7 participants