This is branched from #1258026: Implement userpoint bundles (#2).

The issue title & description is mainly a placeholder until things gel.

CommentFileSizeAuthor
#2 1870674-categorization-as-reference-2.patch21.96 KBmradcliffe
#2 1870674-interdiff-with-1258032-3-do-not-test.patch13.83 KBmradcliffe
#1 issue-1870674-full-diff.patch10.82 KBmradcliffe
#1 issue-1870674-interdiff-with-field-patch-do-not-test.patch2.59 KBmradcliffe

Comments

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
StatusFileSize
new issue-1870674-interdiff-with-field-patch-do-not-test.patch2.59 KB
new issue-1870674-full-diff.patch10.82 KB

Here's a start to this by creating or using an existing a taxonomy term reference field from within userpoints_get_vid(). Not really tested.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 1870674-interdiff-with-1258032-3-do-not-test.patch13.83 KB
new 1870674-categorization-as-reference-2.patch21.96 KB

I really didn't like the whole userpoints_get_vid() thing. It's kind of wonky. I think the following should be easier to create tests:

  • I split out create and get into separate functions.
  • Created an install routine for it if taxonomy is enabled.
  • Created a settings form off of the settings page to create if taxonomy is enabled.

The first patch is a full diff with changes from all issues, and the second patch is a patch branched from #1258032: Allow to attach a points field to any entity and migrate the current hardcoded assignement to users.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

userpoints_get_vid() probably dates back to Drupal 5 or so, way before vocabularies had machine names. I totally agree that it's wonky, machine names should be used for that nowadays. But removing it completely and relying on a machine name instead will require an update function.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs review » Needs work

Any ideas about what to do with the userpoints_get_*_points functions? Can't use aggregation with a field table join without breaking no-SQL compatibility and can't do all of this in PHP/entity loads without running into scalability issues.

The only solution I can see is to mark this "won't fix" and have to live with tid as an entity property.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

Here's a thought after some quick IRC:

field_data_field_userpoints without some columns
delta field_userpoints_points field_userpoints_max_points field_userpoints_tid
0 5 15 3
1 3 5 2
2 20 23 0

Use cases

  1. Get all user's points from field: iterate through $account->field_userpoints['und'] and add points or max_points regardless of tid
  2. Get (un)categorized points via query: query or entityfieldquery for field_userpoints where tid and entity_id are set.
  3. Save transaction: would need to iterate through $account->field_userpoints['und'] to see if tid matches. If it does, then use the current delta, if not, create a new delta and set tid. Is there a race condition here and would it matter?
mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs work » Needs review

Okay, I think the summary from IRC is that

a) ignore this issue for now and look at providing agggreation tools instead of taxonomy terms in userpoints. #1258046: Provide a querying system for userpoints transactions (and totals), #1258052: Provide an easy to extend aggregation API.
b) bundle support on entity #1258026: Implement userpoint bundles
c) add bundle as a field setting to field, and do better field formatting #1258032: Allow to attach a points field to any entity and migrate the current hardcoded assignement to users

marked as needs review for summary review

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Issue summary: View changes

Wrong comment #