Part of #2010184: [meta] convert ‘uri_callback’ entities param to EntityInterface::uri() method

Needs to implement Plugin\Entity\Term.php uri() method and drop taxonomy_term_uri() function replacing it's calls to $term->uri() and clean-up Term annotation uri_callback

Files: 
CommentFileSizeAuthor
#9 taxonomy-uri-2010132-9.patch1.47 KBdanylevskyi
PASSED: [[SimpleTest]]: [MySQL] 57,583 pass(es).
[ View ]
#6 taxonomy-uri-2010132-6.patch1.51 KBdanylevskyi
PASSED: [[SimpleTest]]: [MySQL] 56,042 pass(es).
[ View ]
#3 taxonomy-convert_taxonomy_term_uri-2010132-3.patch1.57 KBInternetDevels
FAILED: [[SimpleTest]]: [MySQL] 55,854 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

Assigned:Unassigned» InternetDevels

We are working today with this issue during Code Sprint UA.

Assigned:InternetDevels» Unassigned
Status:Active» Needs review
StatusFileSize
new1.57 KB
FAILED: [[SimpleTest]]: [MySQL] 55,854 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Patch attached.
All test runs succesfully at local machine.

Status:Needs review» Needs work

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Term.phpundefined
@@ -132,6 +131,20 @@ class Term extends EntityNG implements TermInterface {
   }
+  ¶

https://drupal.org/coding-standards#indenting

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Term.phpundefined
@@ -132,6 +131,20 @@ class Term extends EntityNG implements TermInterface {
+  }
+  ¶

https://drupal.org/coding-standards#indenting

should be right indented due to drupal coding standarts

Assigned:Unassigned» danylevskyi

Assigned:danylevskyi» Unassigned
Status:Needs work» Needs review
StatusFileSize
new1.51 KB
PASSED: [[SimpleTest]]: [MySQL] 56,042 pass(es).
[ View ]

Status:Needs review» Needs work

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Term.php
@@ -134,6 +133,19 @@ public function id() {
+   * Implements Drupal\Core\Entity\EntityInterface::uri().

Should be {@inheritdoc} per the new standard.

Otherwise looks great. Let's get rid of these stinky old uri callbacks for good!

Assigned:Unassigned» danylevskyi

Assigned:danylevskyi» Unassigned
Status:Needs work» Needs review
StatusFileSize
new1.47 KB
PASSED: [[SimpleTest]]: [MySQL] 57,583 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Yes, let's do this.

Status:Reviewed & tested by the community» Needs review

Not sure that right because we break alterability of the uri() (lets see what bot will show for forum tests

Discussion in #2010184: [meta] convert ‘uri_callback’ entities param to EntityInterface::uri() method

Status:Needs review» Needs work

taxonomy_term_uri callback rewrited in forum module to forum_uri.
So seems that this issue is blocked on #1970360: Entities should define URI templates and standard links Assigned to: linclark.
I have added this information to meta issue.

Also seems that this code will be better one

/**
   * {@inheritdoc}
   */
  public function uri(){
    $uri = parent::uri();
    $uri['path'] =  'taxonomy/term/' . $this->id();
    return $uri;
  }

I will also work globally with this at #1970360: Entities should define URI templates and standard links and #2010184: [meta] convert ‘uri_callback’ entities param to EntityInterface::uri() method.
So we needs work here.

Status:Needs work» Needs review

Can someone explain why this should be blocked on the forum issue specifically?
AFAICT forum should still be able to alter in the uri callback in hook_entity_bundle_info_alter() just like before. Just by default no uri callback is used.

@tstoeckler because once term gets overriden uri() method, so nothing is checking a term bundle ‘uri_callback’ override

Status:Needs review» Needs work

You're totally right @andypost. I missed that. That's a super WTF, that the whole uri callback support is in uri() which can be overridden by any entity. All the more reason to get rid of this.

Issue tags:+Needs tests

Yeah we have an issue here, because forum can't sub-class Term and hence inject its own uri for forum vocab terms.
It pains me to say it....but can we have an alter hook or something?
And if tests pass, that means we are missing test coverage for /forum urls (although I'm sure thats not the case).

Can't we just change forum to use url-aliases instead of altering the uri?

OTOH there are probably a bunch of use-cases for uri-altering in contrib...
hmm

Issue summary:View changes

Updated issue summary.