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

fix: minor bug fixes #21

Merged
merged 4 commits into from
Dec 18, 2020
Merged

fix: minor bug fixes #21

merged 4 commits into from
Dec 18, 2020

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Dec 16, 2020 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Fixes two minor bugs:

  1. When syncing metadata, avoid a PHP warning when the single key is not set in the metadata config array. This happens when syncing metadata that supports syncing data from multiple instances of a block, and the metadata config array in the newspack-listings-core.php file doesn't include the `'single' key.
  2. When creating a new list in Query mode, "Listing Type" is now available as a default Sort By option (as "Any" is the default listing type).

Closes #18.

How to test the changes in this Pull Request:

Testing the meta sync bug

  1. Start out on the master branch.
  2. Create a new Event listing post and add an a.) Event Dates block and b.) more than one Jetpack Contact Info block. Fill in data for all blocks.
  3. Attempt to save or publish the post, and observe a PHP warning like PHP Notice: Undefined index: single in /Users/dkoo/Local Sites/newspack/app/public/wp-content/plugins/newspack-listings/includes/newspack-listings-utils.php on line 141 that prevents the post from being saved.
  4. Check out this branch and run npm run build.
  5. Attempt to save or publish the post again, confirm that the PHP warning does not occur and the post is saved successfully. Reload the post and confirm that your data has been actually saved.

Testing the Sort By option

  1. Create a new Curated List block and choose Query mode.
  2. Under Query Settings, expand "Show advanced filters" and observe that the "Sort By" dropdown include a Listing Type option.
  3. Set the "Listing Type" option to something other than "Any" and confirm that "Sort By" option no longer includes the Listing Type option.
  4. Set "Listing Type" to "Any" again and confirm once more that the "Sort By" dropdown include a Listing Type option.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo added the bug Something isn't working label Dec 16, 2020
@dkoo dkoo added this to the Phase 1 (Release) milestone Dec 16, 2020
jeffersonrabb
jeffersonrabb approved these changes Dec 18, 2020
View reviewed changes
includes/newspack-listings-utils.php Outdated
@@ -107,7 +107,7 @@ function get_data_from_blocks( $blocks, $source ) {
}

// If the source has 'single' specified, only get data from the first found block instance.
if ( $source['single'] ) {
if ( isset( $source['single'] ) && ! empty( $source['single'] ) ) {
Copy link

Choose a reason for hiding this comment

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

I think you only need ! empty() here - the isset check is unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and updated in b8a76e0. Guess I wanted to be extra certain :)

@dkoo dkoo merged commit 5f90bc7 into master Dec 18, 2020
@dkoo dkoo deleted the fix/bugfixes branch December 18, 2020 20:53
matticbot pushed a commit that referenced this pull request Dec 18, 2020
@semantic-release-bot
chore(release): 1.1.0 [skip ci]
32eb21a 
# [1.1.0](v1.0.0...v1.1.0) (2020-12-18)

### Bug Fixes

* minor bug fixes ([#21](#21)) ([5f90bc7](5f90bc7))

### Features

* remove borders and padding in editor to match front-end styles ([#14](#14)) ([6c47a17](6c47a17))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Jul 19, 2021
@semantic-release-bot
chore(release): 1.0.0-alpha.1 [skip ci]
77f6a29 
# 1.0.0-alpha.1 (2021-07-19)

### Bug Fixes

* avoid meta sync update error ([#95](#95)) ([cab16aa](cab16aa))
* do not register post-specific sidebars in widgets page ([#93](#93)) ([7716775](7716775))
* editor errors with reusable blocks ([#89](#89)) ([fdc46d3](fdc46d3))
* errors and bugs related to WP 5.8 ([#83](#83)) ([90da6c5](90da6c5))
* force alpha rebuild ([acc2075](acc2075))
* minor bug fixes ([#21](#21)) ([5f90bc7](5f90bc7))
* missing condition for block appender in list container ([#74](#74)) ([2c49896](2c49896))
* syncing attributes from curated list block to inner blocks ([#64](#64)) ([cdbc0bb](cdbc0bb))
* use synced attributes for ListContainer directly ([#73](#73)) ([f8641a7](f8641a7))

### Features

* add block patterns ([#23](#23)) ([a273a40](a273a40))
* bump max number of items per list from 20 to 50 ([#97](#97)) ([009deab](009deab))
* initial post type and block setup ([#1](#1)) ([47dc0c1](47dc0c1))
* listing taxonomies and query mode ([#6](#6)) ([528e1e5](528e1e5))
* more block patterns (real estate, classified ads) ([#84](#84)) ([a51f5af](a51f5af))
* new Curated List block, block pattern, and map functionality ([#3](#3)) ([9be6e7e](9be6e7e))
* remove borders and padding in editor to match front-end styles ([#14](#14)) ([6c47a17](6c47a17))

* v2 release (#85) ([748810d](748810d)), closes [#85](#85) [#40](#40) [#39](#39) [#32](#32) [#41](#41) [#49](#49) [#43](#43) [#56](#56) [#51](#51) [#57](#57) [#61](#61) [#67](#67) [#60](#60) [#70](#70) [#65](#65) [#71](#71) [#66](#66) [#58](#58) [#77](#77) [#81](#81)

### BREAKING CHANGES

* This feature will deprecate existing custom taxonomies, so any existing terms for those taxonomies will be lost.

To fix, we can convert terms from the deprecated taxonomies to standard post categories/tags via a migration script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffersonrabb jeffersonrabb jeffersonrabb approved these changes

Assignees
No one assigned
Labels
bug Something isn't working released [Status] Approved
Projects
None yet
Milestone
Milestone 1: MVP release
Development

Successfully merging this pull request may close these issues.

"Listing Type" is not an available Sort By option at editor page load
3 participants
@dkoo @matticbot @jeffersonrabb