Issue 1498 - Adding entity fields #1526

gkovats · 2024-09-23T13:28:31Z

Closes Add Support for Service Entity Fields in TSML #1498

Adds Entity form fields to Settings page:

5 fields: entity, entity_email, entity_url, entity_phone, entity_location
limits all to 100 characters: wanted to limit because this will add weight to every meeting in every feed.
- Note: the added weight is worth bringing up to Meeting Guide, they'll likely notice
added defaults for entity, entity_url, entity_email, so we start with values from WordPress

Adds Entity fields to export / import

sanitizer caps strings off at 100 characters
export supplies the local entity values for meetings without a data_source, otherwise grabs any entity values from post_meta / meeting fields

Adds Entity fields to TSML Legacy

Added in "Updated" box. I took a crack at it, seemed like it belonged their, but I know we want to be efficient with screen real estate, and it should follow the pattern that TSML UI and Meeting Guide will use. Consider this a draft.

gkovats · 2024-09-23T13:30:15Z

includes/admin_settings.php

@@ -332,7 +346,59 @@ function tsml_settings_page()
                        </form>
                    </div>



Check the order of these on the settings screen. I ended up putting this in spot 2, left column, and moved the Feeds settings to top of the middle column. But I don't stare at this screen much, so it's a guess.

includes/functions.php

- clear meeting cache on entity change - change default entity value check - updated line for returning meeting compare fields

- show default values in form so user is aware what will be in exports - reprocess POT

This reverts commit 45a4fbd.

includes/admin_settings.php

joshreisner

look great, thanks for jumping on this so quickly! just a few comments

includes/functions.php

includes/admin_settings.php

joshreisner · 2024-09-23T15:18:37Z

templates/single-meetings.php

+                                        <?php the_modified_date() ?>
+                                    </p>
+                                    <?php
+                                    if (!empty($meeting->{'data_source'})) {


i think we need some intro text here potentially like "This meeting listing is provided by:"

let's not show the email address - this is more for administrative purposes

the feedback email form should either be hidden and we add something here, or it should send to the entity's feedback_email addresses if present

minor: could we use the $meeting->data_source pattern for this and the other vars?

minor: i would like to steer away from using <br> tags - could this be Ps or a UL?

Adding "This meeting listing is provided by:" only concern it will show up everywhere if we display for local and imported meetings, and it just adds that one more line to what your eye is scanning for

Look correct to you? Should it be smaller?

or it should send to the entity's feedback_email addresses if present

Honestly this raises questions I want to talk about, cause it'll be a mess typing it out. Don't feel equipped with enough history on this tool to answer them myself.

hm, yeah, i'm not so sure. perhaps less is more?

my reason for wanting to add it is that we want to make it clear what this section is - some users might get confused and think it is meeting information.

the purpose of this section is to be like this section in the app:

if we could replicate the headline style for the other sections for the entity name, and make the url and phone buttons like in this pic:

we might try the This meeting listing is provided by: above the headline in a <small>?

here's what it currently looks like in this PR for TSML UI:

not that it needs to match per se but just for context

joshreisner · 2024-09-23T15:22:17Z

includes/admin_settings.php

+
+                            <form method="post" class="stack compact" action="<?php echo $_SERVER['REQUEST_URI'] ?>">
+                                <?php wp_nonce_field($tsml_nonce, 'tsml_nonce', false) ?>
+                                <h3>


could these h3s be labels instead? the smaller type should look good as well

i would keep it Entity Name but change:

Entity Contact Email -> Administrative Contact Email
Entity Contact Phone -> Public Phone Number
Entity Location -> Your Service Area
Entity Website -> Website Address

"Your" felt redundant on looking at it, like it could be in all labels. This OK, or you think the "your" belongs?

that's good! maybe make them labels though, if they're not already

joshreisner · 2024-09-23T15:24:44Z

includes/variables.php

+    'entity_phone',
+    'entity_location',
+    'entity_url',
+];


do we need to add feedback email here? the idea is that they should pass through from the imported service entity

joshreisner · 2024-10-12T16:01:10Z

one minor thing: i'm seeing PHP warnings on some of the entity form fields when running locally:

joshreisner · 2024-10-12T16:07:44Z

also i think feedback_emails should use array_values - see example at https://demo.code4recovery.org/wp-admin/admin-ajax.php?action=meetings

joshreisner · 2024-10-12T16:12:04Z

one last piece of feedback, and feel free to push back and/or say we should make this a subsequent PR, is i think if feedback emails are present on a meeting in legacy, we should expose the feedback form and send to those emails.

ie here:

if (!empty($tsml_feedback_addresses)) { ?>

we might say

if (!empty($tsml_feedback_addresses) || !empty($meeting->feedback_emails)) { ?>

… but looks like you already did the work to make is actually send, so i could be missing something? maybe it's not importing because of the array_values issue above?

- when casting to array, also use array_valus to avoid associative keys - ensure settings page has all possible entity fields present for display - update logic for displaying change request form

gkovats · 2024-10-13T18:10:01Z

@joshreisner FYI

added array_values() to places where I'm casting to array to avoid keys
fixed the admin settings form to ensure those array fields are present before displaying
updated form button to use already existing button style class btn btn-default btn-block and removed my new style
fixed the logic for displaying the change request form - it was entirely an oversight on my part

maybe need a prettier config in this project?

joshreisner · 2024-10-14T02:44:27Z

looks awesome! only detail i noticed is the link icon is a little big - looks like it just needs width and height props

if you don't mind i'll just go ahead and merge this add them in the follow-on PR where i set the new version

gkovats added 4 commits September 22, 2024 22:40

Adding Entity form

3caa62d

Updating saving

5298573

Add entity fields to export / import

ea63f98

Add entity to TSML Legacy

fe12de1

gkovats commented Sep 23, 2024

View reviewed changes

includes/functions.php Show resolved Hide resolved

gkovats added 6 commits September 23, 2024 09:35

POT update

9bf0742

Version bump

45a4fbd

Updates

25ec932

- clear meeting cache on entity change - change default entity value check - updated line for returning meeting compare fields

Cleanup

96e0b97

More cleanup

b51b556

- show default values in form so user is aware what will be in exports - reprocess POT

Revert "Version bump"

0cee6b4

This reverts commit 45a4fbd.

gkovats marked this pull request as ready for review September 23, 2024 14:19

joshreisner reviewed Sep 23, 2024

View reviewed changes

includes/admin_settings.php Outdated Show resolved Hide resolved

joshreisner reviewed Sep 23, 2024

View reviewed changes

gkovats added 8 commits September 24, 2024 18:00

Updates

af635d6

Merge branch 'main' into issue-1498-entity-fields

1987b37

Add feedback_emails to entity export

2679fa8

Swith to field labels

33875e2

Missed a file

e838c6c

Feedback goes to meeting->feedback email if avail

d0289e9

Updating display of entity fields

1b4870a

Style update

6b43e43

Entity updates

4d64152

- when casting to array, also use array_valus to avoid associative keys - ensure settings page has all possible entity fields present for display - update logic for displaying change request form

just noticed this file uses tabs

3501604

maybe need a prettier config in this project?

joshreisner approved these changes Oct 14, 2024

View reviewed changes

joshreisner merged commit 4c37aad into main Oct 14, 2024

joshreisner deleted the issue-1498-entity-fields branch October 14, 2024 02:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 1498 - Adding entity fields #1526

Issue 1498 - Adding entity fields #1526

gkovats commented Sep 23, 2024

gkovats Sep 23, 2024

joshreisner left a comment

joshreisner Sep 23, 2024 •

edited

Loading

gkovats Sep 24, 2024

gkovats Sep 24, 2024

joshreisner Sep 24, 2024

joshreisner Sep 25, 2024

joshreisner Sep 23, 2024

gkovats Sep 24, 2024

joshreisner Sep 24, 2024

joshreisner Sep 23, 2024

joshreisner commented Oct 12, 2024

joshreisner commented Oct 12, 2024

joshreisner commented Oct 12, 2024 •

edited

Loading

gkovats commented Oct 13, 2024

joshreisner commented Oct 14, 2024

		@@ -332,7 +346,59 @@ function tsml_settings_page()
		</form>
		</div>

Issue 1498 - Adding entity fields #1526

Issue 1498 - Adding entity fields #1526

Conversation

gkovats commented Sep 23, 2024

Choose a reason for hiding this comment

joshreisner left a comment

Choose a reason for hiding this comment

joshreisner Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshreisner commented Oct 12, 2024

joshreisner commented Oct 12, 2024

joshreisner commented Oct 12, 2024 • edited Loading

gkovats commented Oct 13, 2024

joshreisner commented Oct 14, 2024

joshreisner Sep 23, 2024 •

edited

Loading

joshreisner commented Oct 12, 2024 •

edited

Loading