Problem/Motivation

The base field definition of various l10n_server entities allows some properties to have NULL as default value. Getter methods for these fields may throw an error when the property has no value (is NULL). See #3311405: Error on translation-history admin page where this was reported for the l10n_server_translation_history entity. This issues addresses the remaining entities.

Steps to reproduce

  • Install l10n_server
  • Investigate the database schema (e.g. "DESCRIBE l10n_server_error")
  • Notice the fields that have both Null set to Yes and Default set to NULL. This issue applies to these field.

Proposed resolution

Same solution as in the mentioned issue

Remaining tasks

Some method's return values are optional e.g. \Drupal\l10n_server\Entity\L10nServerRelease::getDownloadLink this is probably no longer correct. Should we change ?string to string?

CommentFileSizeAuthor
#2 l10n_server-entity-getters-3347271-2.patch6.84 KBsutharsan

Comments

Sutharsan created an issue. See original summary.

sutharsan’s picture

sutharsan commented
Status: Active » Needs review
StatusFileSize
new l10n_server-entity-getters-3347271-2.patch6.84 KB
sutharsan’s picture

sutharsan commented
fmb’s picture

fmb commented
Status: Needs review » Needs work

Thanks! @nicoloye and I talked about this, and we think in most cases, unless for instance when we return a count, we should return NULL if the field is empty. So we should make the return type nullable, and we could use something like the null safe operator since we are running PHP > 8.0.

  • FMB committed f481233d on 3.0.x 
    Issue #3347271 by Sutharsan, FMB, nicoloye: Prevent error on entity...

FMB credited nicoloye.

fmb’s picture

fmb commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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