Describe your bug or feature request.

Currently, all of our methods for getting the completed, placed and created timestamps can return strings (even though the documented return type is int (See the example below):

  /**
   * Gets the order placed timestamp.
   *
   * @return int
   *   The order placed timestamp.
   */
  public function getPlacedTime();
drush ev "var_dump(\Drupal\commerce_order\Entity\Order::load(120)->getPlacedTime())"
string(10) "1752501150"

Also, we need to specify that NULL can be returned and explicitly return NULL when the field is empty.

Let's also return NULL for various other fields for which we aren't explicitly returning NULL (the total price, the email and the IP address for example.

Issue fork commerce-3536290

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

Title: Order::getPlacedTime() should cast the return value to an int » The created, placed and completed timestamps should return integers, and can return NULL
zaporylie’s picture

Let's target both getPlacedTime and getCompletedTime. Not sure about setting the return type on the interface though - I am all for having it, but this will break things for those who override/decorate the entity class

jsacksick’s picture

Title: The created, placed and completed timestamps should return integers, and can return NULL » Strict typing for order getters and setters

Actually, more in favor of increasing the scope and doing this once for all... This change is long overdue and should have been part of 3.0.0

jsacksick’s picture

Title: Strict typing for order getters and setters » Strict typing for Order getters and setters
jsacksick’s picture

Title: Strict typing for Order getters and setters » Make sure getters for timestamp fields return integers
jsacksick’s picture

Issue summary: View changes

  • jsacksick committed fa73d2ae on 3.x
    Issue #3536290 by jsacksick, zaporylie: Make sure getters for timestamp...
jsacksick’s picture

Status: Active » Fixed

Ok the change shouldn't be disruptive as I finally skipped return types, will go ahead and merge this.

  • jsacksick committed fd969106 on 3.x
    Issue #3536290 followup by jsacksick: Cast more timestamp fields and...
ivnish’s picture

Status: Fixed » Closed (fixed)

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