Problem/Motivation

I'm not sure that the calculated field Declaration:status is going to work. The value depends on the date, so it would break any caching.

Proposed resolution

Options

  1. I'm not sure we need it: the function getStatus() is fine for most purposes.
  2. We could appropriate cache expiry to the entity. I guess the purpose of it was for display (a computed field being more elegant than hook_entity_extra_field), but I'm open to other approaches to displaying status.

Also the internal values are printed - we need translated UI text instead.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork gift_aid-3505393

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:

About issue forks

Comments

adamps created an issue. See original summary.

adamps’s picture

adamps commented
Assigned: Unassigned » adamps
adamps’s picture

adamps commented
Assigned: adamps » Unassigned
adamps’s picture

adamps commented
Issue summary: View changes
adamps’s picture

adamps commented

This is presumably a beta-blocker, so I'll comment on it to bump it up the list.

jonathanshaw made their first commit to this issue’s fork.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

I suggest we remove all code related to declaration status for now. I'm not clear enough what problem this is trying to solve, to know what constraints the solution needs to obey. I suspect that when actual problems emerge that need something like this, the solution(s) we end up with might be quite different.

The code we can remove is:
Declaration::statusOptions()
Declaration status field
DeclarationInterface status constants
Drupal\gift_aid\Plugin\Field\DeclarationStatusItemList

adamps’s picture

adamps commented

We are using the status in several places. However AFAICS we don't anywhere rely on it being a field. Here's a MR that tests removing the field but keeping the functions.

adamps opened merge request !18

adamps’s picture

adamps commented
Title: Declaration:status problem » Remove declaration status computed field
Category: Bug report » Task
adamps’s picture

adamps commented
Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • adamps committed e05b5a21 on 1.x 
    #3505393 Remove declaration status computed field

Status: Fixed » Closed (fixed)

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