Cleanups

Here is output from a search for @todo

/home/adam/drupal/gift_aid/gift_aid.module:
   55   */
   56  function gift_aid_entity_extra_field_info() {
   57:   // @todo use computed fields not extra fields.
   58    // Although the information is computed from other entities so maybe not appropriate??
   59    $extra['gift_aid_donor']['gift_aid_donor']['display']['status'] = [

This is your comment and my response. I believe that a computed field would cause caching problems because it would end up making data from other entities (which may change independently at any time) part of the storage for this entity. It would potentially also cause performance problems because it would greatly increase the storage of the donor entity to include all declarations/cancellations.

/home/adam/drupal/gift_aid/src/Controller/ContextOverviewController.php:
   50  
   51      if (!$donor->isCurrentUser()) {
   52:       // @todo For admin, show a form for creating a cancellation.
   53        return NULL;
   54      }

This is non-essential. The staff/admin don't have any natural way of reaching this page - they would have to just type in the URL or perhaps the Donor emails the link to them.

/home/adam/drupal/gift_aid/src/Declaration/DeclarationViewsData.php:
   16      $data = parent::getViewsData();
   17  
   18:     // @todo Needs writing.
   19      return $data;
   20    }

If you want custom views handling then it needs writing. Could convert to an issue I guess.

/home/adam/drupal/gift_aid/src/Donor/DonorAccessControlHandler.php:
   31  
   32        case 'delete':
   33:         // @todo Block if there are declarations.
   34      }
   35  

Only admin can delete anyway. Yes, we could do that. It could prevent the admin making a big mistake. It could also force them to tediously delete declarations one-by-one in the case they know what they are doing. E.g. a GDPR deletion request. So arguments either way perhaps.

/home/adam/drupal/gift_aid/src/Entity/Declaration.php:
  254     */
  255    public function getStatus() {
  256:     // @todo provide a documented and centralized source of truth for declaration status semantics.
  257      if ($this->getValidity() == DeclarationInterface::DECLARATION_INVALID) {
  258        return DeclarationInterface::DECLARATION_INVALID;

This is your comment. This function could be seen as the centralised source. However it seems like you prefer somewhere else.

/home/adam/drupal/gift_aid/src/Entity/DeclarationTypeInterface.php:
   12  interface DeclarationTypeInterface extends ConfigEntityInterface, EntityDescriptionInterface, RevisionableEntityBundleInterface {
   13  
   14:   // @todo document significance of the methods.
   15    const METHOD_ORAL = 'oral';
   16    const METHOD_WRITTEN = 'written';

Yes we should have a comment here.

/home/adam/drupal/gift_aid/src/Entity/RecordBase.php:
  478     * A BC shim for getOriginal introduced in D11.2.
  479     *
  480:    * @todo Remove this once we drop support for <D11.3.
  481     */
  482    public function getOriginal() : ?static {

This is a legit @todo - a task we can't do now, and wish to do later.

/home/adam/drupal/gift_aid/src/Form/RecordForm.php:
  140      }
  141  
  142:     // @todo Show/hide Start and End dates depending on the DateBased state.
  143      /*    $date_based_checked = [
  144      ':input[name="date-based[value]"]' => ['checked' => TRUE],

Perhaps we can just delete this commented out code? We don't seem likely to care about non-date-based any time soon.

/home/adam/drupal/gift_aid/src/Plugin/views/field/DeclarerViewsField.php:
   47    }
   48  
   49:   // @todo Delete whole file or put code below in DonorViewsData?
   50    // Override the Declarer view field for the Declaration entity.
   51    // $data['gift_aid_declaration_field_data']['context'] = [

If you want custom views handling then it needs writing. If not then just delete. Could convert to an issue I guess, same as above.

/home/adam/drupal/gift_aid/tests/src/Functional/DeclarationFormTest.php:
  213  
  214      // Find cancel link.
  215:     // @todo Simplify once https://www.drupal.org/project/symfony_mailer/issues/3570640 is released.
  216      $dom = new \DOMDocument();
  217      $dom->loadHTML($this->email->getHtmlBody());

This is a legit @todo - a task we can't do now, and wish to do later.

/home/adam/drupal/gift_aid/tests/src/Kernel/DeclarationTest.php:
   21  class DeclarationTest extends GiftAidTestBase {
   22  
   23:   // @todo test the revision user and date on a newly created declaration.
   24    use NodeCreationTrait;
   25  

This is a possible missing test. Could remove the comment and add to the testing issue if you prefer.

/home/adam/drupal/gift_aid/tests/src/Kernel/DonorTest.php:
   86      $this->assertFalse($dd->access('update', $nobody));
   87  
   88:     // @todo Block deleting if there are declarations.
   89      $this->assertTrue($dd->access('delete', $admin));
   90      $this->assertFalse($dd->access('delete', $staff));

This is the testing version of a @todo above.