Issue 1500 conference validation #1512

gkovats · 2024-09-11T00:31:12Z

For issue #1500 :
The current form has 1 variable to track valid state for the whole form based on formatted_date, so tracking a second field meant tracking state per field. Also just built a $fields object to get all the field selectors up front. Didn't know how far this would go.

conference_url field state now tracked separately, and validated on page load, so meetings with currently bad Zoom urls will either throw an error / highlight the field and disable publish, or automatically correct the url format.
moved one formatted_address field error state to this new method, "In person meetings must have a specific address"
kept language for error messages in PHP so they can use gettext translation.

Testing:

load main branch, and add a poorly formatted zoom url, then switch to this branch and load the meeting. The Zoom url should get validated immediately - if bad, will error and block publishing. If close, will get corrected and message will say "Url was cleaned up.."
Updated pointers for Open / Closed checks, so make sure you can't click both types
Test the validation on Formatted address:
- When online meeting, a full address should show the message "Full addresses will trigger meeting is closed message"
- When in person meeting, an approximate address should throw an error "must have a full address for in person meetings" and block publishing

gkovats · 2024-09-11T00:31:45Z

assets/src/admin-meeting.scss

@@ -98,6 +98,17 @@
 		}
 	}



used <small> tags to follow input fields for immediate error messaging.

👍 thanks for using the same color of red we use in other places - maybe eventually we can use css vars for it

gkovats · 2024-09-11T00:33:09Z

includes/admin_meeting.php

+            </small>
+            <small class="warning_message" data-message="2">
+                <?php _e('Warning: Unable to process this address for exact location.', '12-step-meeting-list') ?>
+            </small>


I added this error state to capture when we get an error from the geocoding call. I figured it was worth displaying, but you may know reasons why this isn't useful - maybe frequent errors?

no this is great - we can now cross this requirement off our refactor geocoding ticket

gkovats · 2024-09-11T00:33:55Z

package.json

+    "scripts": {
+        "start": "npx mix watch",
+        "build": "npx mix --production"
+    },


Just added npm start as a shortcut for mix watch. I'm lazy.

sounds good - we can make make this standard in other places too

gkovats · 2024-09-11T00:35:27Z

assets/src/admin.js

@@ -60,26 +60,86 @@ jQuery(function ($) {
 		//make sure geocoding is finished (basic form validation)
 		var form_valid = true;



Sorry, this is where I went nuts. Made an object to track all the fields, then a small jQuery function to track / attach "state" to each field, and an upper formstate check, so formatted_address and conference_url (and other fields) could have issues independently.

gkovats · 2024-09-11T00:37:22Z

assets/src/admin.js

+                    pendingFields.push($fields[field][0])
+                }
+            });
+            form_valid = !pendingFields.length;


later, we could use pendingFields as an array of DOM elements for hyperlinks, to anchor link users to in some top message. "The following fields need attention: ...."

gkovats · 2024-09-11T00:37:59Z

assets/src/admin.js

-		});
-		$('input#conference_phone').change(function () {
-			$('input#formatted_address').change();
+


Zoom error and cleanup logic

including demo url in case format failure isn't clear

kiyote33

Good job George. Like the radio button effect on the open/close. Nice touch. Everything worked as described in your prep notes. One little thing with the Full Address warning. Shouldn't that be an error instead of warning? You can't really proceed until you fix. Other than that, I think it's good to go.

joshreisner · 2024-09-11T13:23:58Z

i'm really looking forward to reviewing this after NAATW, thanks @gkovats !

gkovats · 2024-09-12T19:58:37Z

Just a heads up, if you're ever waiting on me to merge a PR, let me know - I won't unless asked. Not my codebase, still new.

gkovats · 2024-09-12T21:59:52Z

@kiyote33 Thanks, I missed that - it was previously an error, but I didn't treat it as such. Just updated in the latest commit.

joshreisner

works great, and thanks for rebuilding the strings

the only problem i had while testing was my old js and css assets were cached, which meant that i saw the error messages on the screen as black text flush left on the page

i am 99% sure that when this gets released our TSML_VERSION bump will bust the cache but it's just something to keep an eye on when this goes out

joshreisner · 2024-09-16T14:16:47Z

assets/src/admin-meeting.scss

@@ -98,6 +98,17 @@
 		}
 	}



👍 thanks for using the same color of red we use in other places - maybe eventually we can use css vars for it

joshreisner · 2024-09-16T14:19:00Z

includes/admin_meeting.php

+            </small>
+            <small class="warning_message" data-message="2">
+                <?php _e('Warning: Unable to process this address for exact location.', '12-step-meeting-list') ?>
+            </small>


no this is great - we can now cross this requirement off our refactor geocoding ticket

joshreisner · 2024-09-16T14:20:35Z

package.json

+    "scripts": {
+        "start": "npx mix watch",
+        "build": "npx mix --production"
+    },


sounds good - we can make make this standard in other places too

gkovats added 4 commits September 10, 2024 07:21

Updating package scripts, first JS pass

83afb36

Building field state and form state

6f6291c

Adding zoom url cleanup

509566b

Updated POT file, and added to readme

46283ba

gkovats commented Sep 11, 2024

View reviewed changes

gkovats requested a review from joshreisner September 11, 2024 00:38

Adjusting zoom error message

66fa951

including demo url in case format failure isn't clear

kiyote33 self-requested a review September 11, 2024 12:51

kiyote33 approved these changes Sep 11, 2024

View reviewed changes

Changing formatted_address item to error

c81bbd8

joshreisner approved these changes Sep 16, 2024

View reviewed changes

joshreisner merged commit 7497aa8 into main Sep 16, 2024

joshreisner deleted the issue-1500-conference-validation branch September 16, 2024 14:24

joshreisner mentioned this pull request Sep 22, 2024

Tighten up "Confererence URL" validation #1500

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 1500 conference validation #1512

Issue 1500 conference validation #1512

gkovats commented Sep 11, 2024 •

edited

Loading

gkovats Sep 11, 2024

joshreisner Sep 16, 2024

gkovats Sep 11, 2024 •

edited

Loading

joshreisner Sep 16, 2024

gkovats Sep 11, 2024

joshreisner Sep 16, 2024

gkovats Sep 11, 2024

gkovats Sep 11, 2024

gkovats Sep 11, 2024

kiyote33 left a comment

joshreisner commented Sep 11, 2024

gkovats commented Sep 12, 2024

gkovats commented Sep 12, 2024 •

edited

Loading

joshreisner left a comment

joshreisner Sep 16, 2024

joshreisner Sep 16, 2024

joshreisner Sep 16, 2024

		@@ -60,26 +60,86 @@ jQuery(function ($) {
		//make sure geocoding is finished (basic form validation)
		var form_valid = true;

Issue 1500 conference validation #1512

Issue 1500 conference validation #1512

Conversation

gkovats commented Sep 11, 2024 • edited Loading

Testing:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkovats Sep 11, 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

kiyote33 left a comment

Choose a reason for hiding this comment

joshreisner commented Sep 11, 2024

gkovats commented Sep 12, 2024

gkovats commented Sep 12, 2024 • edited Loading

joshreisner left a 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

gkovats commented Sep 11, 2024 •

edited

Loading

gkovats Sep 11, 2024 •

edited

Loading

gkovats commented Sep 12, 2024 •

edited

Loading