stream icon indicating copy to clipboard operation
stream copied to clipboard

Use unique keys for stream meta

Open nprasath002 opened this issue 6 years ago • 7 comments
trafficstars

Right now stream use the same key if meta has an array. See the screenshot below. Stream usesuser_meta as key for all the data in array user_meta instead of array keys

Screen Shot 2019-10-16 at 3 26 44 PM

image

Checklist

  • [ ] Project documentation has been updated to reflect the changes in this pull request, if applicable.
  • [ ] I have tested the changes in the local development environment (see contributing.md).
  • [ ] I have added phpunit tests.

Release Changelog

  • Fix: Describe a bug fix included in this release.
  • New: Describe a new feature in this release.

Release Checklist

  • [ ] This pull request is to the master branch.
  • [ ] Release version follows semantic versioning. Does it include breaking changes?
  • [ ] Update changelog in readme.txt.
  • [ ] Bump version in stream.php.
  • [ ] Bump Stable tag in readme.txt.
  • [ ] Bump version in classes/class-plugin.php.
  • [ ] Draft a release on GitHub.

Change [ ] to [x] to mark the items as done.

nprasath002 avatar Oct 16 '19 09:10 nprasath002

This looks great, thank you so much for the contribution @nprasath002!

On a sidenote -- are you using the bundled development environment to get Xdebug working with the plugin or you have custom setup for that?

kasparsd avatar Oct 16 '19 11:10 kasparsd

https://github.com/Chassis/Chassis comes with xdebug

nprasath002 avatar Oct 17 '19 02:10 nprasath002

@nprasath002 what if there's a meta key name conflict across object types.. is that possible?

Here's my live db meta key breakdown for reference, sorted by COUNT DESC. It's true that user_meta definitely takes the cake.

How is user_meta data read back? If we rename the keys, are the display operations still working fine?

meta_key | Count
----------------
user_meta | 37027773
post_title | 7006941
singular_name | 3911511
post_id | 3097690
comment_type | 3097611
user_name | 3007114
user_id | 1865416
comment_status | 1168822
is_spam | 1140055
display_name | 420376
option | 253911
label | 253843
context | 253701
value | 248995
new_status | 248243
old_status | 248243
post_date | 245711
post_date_gmt | 245711
old_value | 232780
option_key | 204325
revision_id | 66936
form_id | 41335
form_title | 40949
lead_id | 40829
roles | 12877
name | 10031
email | 8269
url | 6105
parent_id | 5992
parent_title | 4828
meta_key | 4655
post_type | 4655
meta_value | 4622
user_login | 4597
id | 1437
slug | 1430
type | 1402
success | 1233
version | 1233
new_status_name | 1225
old_status_name | 1225
title | 1207
taxonomy | 1135
taxonomy_label | 1135
term_id | 1135
term_name | 1135
term_parent | 1133
old_version | 1125
action | 1120
lead_status | 851
old_instance | 656
menu_id | 600
menu_data | 580
is_new | 317
status | 275
prev | 258
confirmation | 212
page | 169
sidebar_id | 164
widget_id | 164
section | 141
tab | 141
sidebar_name | 138
form_status | 110
filename | 80
notification | 59
new_value | 50
option_title | 50
new_role | 47
old_role | 47
is_update | 44
author | 28
item_id | 28
ids | 25
count | 20
titles | 20
note_id | 15
order_id | 15
location | 6
location_id | 6
tax_rate | 5
auto_updated | 4
new_version | 4
page_id | 4
tax_rate_country | 4
tax_rate_name | 4
tax_rate_state | 4
attribute_label | 3
attribute_name | 3
attribute_orderby | 3
attribute_public | 3
attribute_type | 3
end_date | 3
start_date | 3
tax_rate_class | 3
tax_rate_compound | 3
tax_rate_order | 3
tax_rate_priority | 3
tax_rate_shipping | 3
error | 2
args | 1
event_type | 1

lkraav avatar May 04 '20 07:05 lkraav

@eugenekireev seems to have worked on it way back in the day in https://github.com/xwp/stream/commit/452d6e162c9c61442af6fb78792bc54a7579ec4d

I wonder if he remembers anything about the why's?

lkraav avatar May 04 '20 08:05 lkraav

Now that #1080 is merged, I'll take care of this next.

lkraav avatar Mar 08 '22 11:03 lkraav

I've looked into this. It seems $user_meta is a special object, and I think we need to continue giving it special treatment to the end.

Stream meta keys don't don't really lend themselves for de-duplication... I think the meta key will simply have to be array notation string, like user_meta[user_role_label] etc.

If no objections @kasparsd @kopepasah I'll file a PR towards this.

lkraav avatar Mar 09 '22 14:03 lkraav

PR filed.

I don't see how generic user_meta meta keys are useful in any way, since you can't query for values based on anything, so we should probably do something here.

I left original (array) conversions as-is for the final record insert, because maybe users are relying on ability to pass arrays in.

Also @kasparsd develop branch isn't up to date with master.

lkraav avatar Mar 09 '22 16:03 lkraav