[wp-trac] [WordPress Trac] #40702: Add WordCamps and meetup events to the News Dashboard widget
WordPress Trac
noreply at wordpress.org
Thu May 11 18:08:39 UTC 2017
#40702: Add WordCamps and meetup events to the News Dashboard widget
-------------------------------------------------+-------------------------
Reporter: iandunn | Owner:
Type: enhancement | Status: new
Priority: normal | Milestone: 4.8
Component: Administration | Version: trunk
Severity: normal | Resolution:
Keywords: needs-dev-note has-patch needs- | Focuses:
unit-tests dev-feedback | javascript, rest-api
-------------------------------------------------+-------------------------
Comment (by iandunn):
`40702.6.diff` tests pretty good for me, but I did find two bugs:
1. The wrong template is shown when searching for a location that wasn't
found (e.g., `narnia`). `templateParams.unknownCity` should be set, so
that the correct template gets rendered. Instead, it's showing the normal
template, but the city name is missing: `Attend an upcoming event near .
`
1. The same thing happens when no location is saved, and api.w.org returns
`no_location_found`. In this case, though, no error should be shown at
all, since it was an automatic request.
Lines `307-320` in the patch remove the code that codes both of those
situations.
''Side note: I've been
[https://gist.github.com/iandunn/15a5fdaa78d4bfcd2ce29e0453887d7f using a
`pre_http_request` filter to easily test error conditions]. That might
come in handy to others. That wouldn't help with either of the two bugs
above, though.''
I share some of Ozz's concerns, particularly about exposing a user's
location to admins. There are some situations where the admin will already
have access to that data, but many others where they wouldn't. There will
probably be at least a small number of situations where the user would not
feel comfortable sharing their location. Since there doesn't seem to be
any specific use case for exposing that data, it seems like it should be
removed.
cc @adamsilverstein, in case you have any thoughts on leveraging `wp.api`
here.
== Minor nitpicky stuff ==
* `dashboard.js` - Grouping `requestParams._embed = 1;` with the other
`requestParams` statements seems like it would improve readability
* `get_item_for_user_id()` - `$data = array()` is not needed, it gets
overridden immediately
* `prepare_item_for_response()` - I think The `description`, `country`,
etc vars would be more readable if the `?` and `:` were aligned
* `prepare_item_for_response()` returns an array, but the docblock says
it'll be a `WP_REST_Response`
* `get_item_permissions_check()` is 95% duplicated in both controllers.
Would it be better to make it that DRY? Or at least leave a comment in
both that changes to either might need to be synced?
--
Ticket URL: <https://core.trac.wordpress.org/ticket/40702#comment:27>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list