Once #3077598: Add a service to get user information from d.o is done, we should make that data available through computed fields or properties on the username field.

CommentFileSizeAuthor
#11 create_computed_fields-3077599-11.patch4.62 KByonas.legesse
#11 interdiff-9-11.txt4.44 KByonas.legesse
#9 create_computed_field_properties_for_user_information-3077599-9.patch7.51 KByonas.legesse
#6 Create_computed_field_properties_for_user_information-3077599-6.patch5.08 KByonas.legesse
#5 Create_computed_field_properties_for_user_information-3077599-5.patch5.14 KByonas.legesse

Comments

hussainweb created an issue. See original summary.

hussainweb’s picture

hussainweb
Primary language English
commented
Related issues: +#3077598: Add a service to get user information from d.o
hussainweb’s picture

hussainweb
Primary language English
commented

Reference documentation: https://www.drupal.org/docs/8/api/entity-api/dynamicvirtual-field-values...

yonas.legesse’s picture

yonas.legesse commented

More clarification as to which fields need to be included. The current response includes more than 40 field elements.

yonas.legesse’s picture

yonas.legesse commented
Status: Active » Needs work
StatusFileSize
new Create_computed_field_properties_for_user_information-3077599-5.patch5.14 KB

I've included starter computed fields assuming the service patch submitted in #3077598 is okay to continue with.

yonas.legesse’s picture

yonas.legesse commented
Status: Needs work » Needs review
StatusFileSize
new Create_computed_field_properties_for_user_information-3077599-6.patch5.08 KB

@hussainweb, i have incorporated the changes made in issue #3077598. This is meant to serve as a starter. Upon review, indicate what is exactly expected as well. P.S I've altered t() calls to $this->t().

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs review » Needs work

Some comments. Also, I committed #3077598: Add a service to get user information from d.o with a different method name, so that needs to be changed as well.

  1. +++ b/src/DOComputedFields.php
@@ -0,0 +1,51 @@
+    switch ($field_type) {

    Let's add at least these fields:
    - Bio
    - IRC Nick
    - Websites
    - Country
    - Languages
    - First Name
    - Last Name
    - URL
    - Created

    We can even do some of these in other issues. For example, "Industries" requires another API request, so it might make sense to do that in a different issue. 

  2. +++ b/src/DOComputedFields.php
@@ -0,0 +1,51 @@
+        return $userInformation->field_industries_worked_in;
+
+      break;

    `break` is not required here. Nor in the next `case`.

hussainweb’s picture

hussainweb
Primary language English
commented

Also, let's not worry about the formatters here. I don't think we'll add those formatters at all. The computed fields are meant to be used with modules like views.

yonas.legesse’s picture

yonas.legesse commented
Status: Needs work » Needs review
StatusFileSize
new create_computed_field_properties_for_user_information-3077599-9.patch7.51 KB

I've added the suggested fields and changed t() calls to $this->t().

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs review » Needs work

Thanks again for all the work. I have a few more comments.

  1. +++ b/src/DOComputedFields.php
@@ -0,0 +1,74 @@
+    $this->service = \Drupal::service('do_username.user_service');

    What is `$this->service`? Also, let's give a better name. How about something like `doUserService`? 

  2. +++ b/src/DOComputedFields.php
@@ -0,0 +1,74 @@
+        return date('m/d/Y H:i:s', $userInformation->created);

    I am thinking lets just return the timestamp or something machine-readable like `date('c', ...)` 

  3. +++ b/src/Plugin/Field/FieldType/DrupalOrgUsernameItem.php
@@ -81,10 +120,10 @@ class DrupalOrgUsernameItem extends FieldItemBase {
-      '#title' => t('Maximum length'),

    I'd rather do this in a separate issue.

yonas.legesse’s picture

yonas.legesse commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-9-11.txt4.44 KB
new create_computed_fields-3077599-11.patch4.62 KB

I've addressed the comments. However, need some clarification for 1 : "What is `$this->service`? " I've changed the name as per your suggestion but somehow feel there's more. Please clarify if the current patch has missed anything.

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs review » Fixed

LGTM. Let's add tests in a follow-up (#3143091: Add tests for computed fields).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.