Added required to the required fields #216

mehul0810 · 2023-01-29T03:22:21Z

Description of the Change

I have added (required) to the podcast meta fields to ensure that it passes the podcast feed while submission.

Closes #63

Alternate Designs

Possible Drawbacks

Verification Process

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my change.
All new and existing tests passed.

Changelog Entry

Credits

Props @

cadic

@mehul0810 thank you for this PR.

The current change only adds "(required)" suffix to a few fields. The validation is not performed, so we are still able to save the Podcast with empty "Author", "Summary" and "Cover image" fields.

Could we add validation to both the browser and back-end before saving and provide an error message if any of the required fields are submitted empty?

cadic

@mehul0810 thanks for updating this.

Looks like that with all recent changes, we have lot of things broken on the site:

The onboarding workflow fails at step 1 (Could not insert the podcast term)
The Quick-add from the podcasts sidebar panel doesn't work (do nothing without any error)
Most of E2E tests fail (the test suite fails to create the podcast)

So this PR needs to be updated:

Fix the onboarding process
Fix E2E tests — it should also include all required fields when creating the test podcast
Need to decide what to do with the quick-add interface from the sidebar

nateconley · 2023-03-04T03:08:59Z

@cadic I pushed some changes to update the onboarding page with this functionality and use the new validation logic on that page.

I updated the e2e tests. They work locally, but they are failing CI. Beside adding content to the required fields, I have added a media upload (using the 10up util) before each cy.createTerm call.

I did not update the block quick add. As I see it, there are three options:

Remove this functionality and point to the taxonomy admin screen
Add the required fields, media upload, and validation to the block
Allow terms to be created without required fields from this location

jeffpaul · 2023-03-06T15:56:10Z

@nateconley I think I would prefer option 2 as that allows an editor to stay within the block editor construct and add a new Podcast without having to leave the post and navigate into the plugin settings

includes/admin/onboarding.php

Spoygg · 2023-03-08T12:36:02Z

@jeffpaul I've added server side validation for onboarding step 1 and it also works on new term form.

ggutenberg

Found a couple of issues, as well as a bunch of tests continuing to fail. I have some time this week to look into these issues.

includes/datatypes.php

ggutenberg · 2023-03-29T19:36:39Z

includes/datatypes.php

+ *
+ * @return string
+ */
+function validate_taxonomy_fields( $term, $taxonomy, $args ) {


On edit-tags.php?taxonomy=podcasting_podcasts&podcasts=true, if this function throws an error, it does not save selections for the following:

Cover image

Category 1-3

On admin.php?page=simple-podcasting-onboarding&step=1 if it throws a Cover image error, it resets the entire form.

jeffpaul · 2023-04-03T14:01:38Z

@10up/open-source-practice FYI that this is back up for review, thanks!

peterwilsoncc

I've added a few notes inline, unslashing data and the accessibility items are the most important.

I've used 🔢 to indicate the translation changes apply to multiple lines.

includes/admin/onboarding.php

includes/admin/views/onboarding-page-one.php

includes/datatypes.php

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

includes/datatypes.php

jeffpaul · 2023-05-08T19:18:27Z

@faisal-alvi some updates here during your OSS time could help get this merged into the next release, thanks!

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

# Conflicts: # package-lock.json # package.json

peterwilsoncc

LGTM and tests well.

I pushed a few commits to

add a for attribute to the show name and category labels (& related ID on the input)
add aria-describedby attributes to each of the fields
fix a typo
resolve merge conflict with develop.

update label for required fields

5daeeed

mehul0810 requested a review from cadic as a code owner January 29, 2023 03:22

mehul0810 mentioned this pull request Jan 30, 2023

Mark any required fields when adding/editing a podcast feed #63

Closed

cadic assigned mehul0810 Jan 30, 2023

cadic suggested changes Jan 30, 2023

View reviewed changes

jeffpaul added this to the 1.5.0 milestone Jan 30, 2023

mehul0810 added 2 commits January 31, 2023 16:47

add validation for required podcasts field

6235cc6

improve validation for podcast fields

0541a6f

jeffpaul requested a review from cadic February 6, 2023 17:03

cadic suggested changes Feb 13, 2023

View reviewed changes

roshniahuja self-assigned this Feb 22, 2023

Validate onboarding and e2e tests

4d45ce2

Spoygg reviewed Mar 7, 2023

View reviewed changes

includes/admin/onboarding.php Outdated Show resolved Hide resolved

Ivan Ivanić added 2 commits March 8, 2023 13:32

Add server side validation to onboarding step 1

4036df4

Fix typo

0061620

Clean up

26dd7f7

jeffpaul requested review from a team and Sidsector9 and removed request for a team March 20, 2023 13:57

ggutenberg requested changes Mar 29, 2023

View reviewed changes

Garth Gutenberg added 2 commits March 30, 2023 07:49

Show Category 1 as required

1d8388d

Fix tests

ffd1024

ggutenberg requested a review from a team as a code owner March 30, 2023 14:01

Fix tests on WP < 6.1

efe31f0

jeffpaul removed the request for review from a team April 3, 2023 14:00

peterwilsoncc requested changes Apr 4, 2023

View reviewed changes

Sidsector9 and others added 3 commits April 14, 2023 15:55

Update includes/admin/onboarding.php

eedb30a

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

Update includes/datatypes.php

2e8d8a8

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

Update includes/datatypes.php

e0114b7

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

ravinderk reviewed Apr 14, 2023

View reviewed changes

includes/datatypes.php Outdated Show resolved Hide resolved

faisal-alvi and others added 5 commits May 10, 2023 18:30

using parse_str

74db92e

adding text domain

558766c

adding aria-describedby for the descriptive text

d3acfb5

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

adding aria-describedby for the descriptive text

6105d35

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

adding aria-describedby for the descriptive text

127ddb1

Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>

faisal-alvi requested a review from peterwilsoncc May 10, 2023 13:07

jeffpaul requested a review from iamdharmesh May 15, 2023 16:02

peterwilsoncc added 5 commits May 22, 2023 13:28

Merge branch 'develop' into fix/63-required-fields

6c3b264

# Conflicts: # package-lock.json # package.json

Relate lable to podcast name field.

f03f62a

Add aria-describedby attribute to input fields.

b7efe5d

Add missing quote mark to ID attribute.

b422d8b

Relate label to podcast category dropdown.

074fa18

peterwilsoncc approved these changes May 22, 2023

View reviewed changes

jeffpaul merged commit d915c8e into develop May 22, 2023
8 checks passed

jeffpaul deleted the fix/63-required-fields branch May 22, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added required to the required fields #216

Added required to the required fields #216

mehul0810 commented Jan 29, 2023

cadic left a comment

cadic left a comment

nateconley commented Mar 4, 2023

jeffpaul commented Mar 6, 2023

Spoygg commented Mar 8, 2023 •

edited

ggutenberg left a comment

ggutenberg Mar 29, 2023

jeffpaul commented Apr 3, 2023

peterwilsoncc left a comment

jeffpaul commented May 8, 2023

peterwilsoncc left a comment

Added required to the required fields #216

Added required to the required fields #216

Conversation

mehul0810 commented Jan 29, 2023

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Checklist:

Changelog Entry

Credits

cadic left a comment

Choose a reason for hiding this comment

cadic left a comment

Choose a reason for hiding this comment

nateconley commented Mar 4, 2023

jeffpaul commented Mar 6, 2023

Spoygg commented Mar 8, 2023 • edited

ggutenberg left a comment

Choose a reason for hiding this comment

ggutenberg Mar 29, 2023

Choose a reason for hiding this comment

jeffpaul commented Apr 3, 2023

peterwilsoncc left a comment

Choose a reason for hiding this comment

jeffpaul commented May 8, 2023

peterwilsoncc left a comment

Choose a reason for hiding this comment

Spoygg commented Mar 8, 2023 •

edited