Problem/Motivation

See title

Proposed resolution

1) Fix warnings
2) Fix some @todo
3) Remove some redundant code:

  • modules/gift_aid_commerce/src/EventSubscriber/GiftAidRelatedContextsSubscriber.php
  • parts of src/Declaration/DeclarationStorage.php for related contexts
  • NB The related context manager remains and could be re-added as described in a bullet in the roadmap.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork gift_aid-3535869

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

adamps created an issue. See original summary.

adamps’s picture

Assigned: Unassigned » jonathanshaw
Issue summary: View changes
Status: Active » Needs review

Now have a triple green on the test results😃. OK with you?

I have never installed the necessary for infrastructure for FunctionalJavascript tests. Probably we need just one test, so it's not really efficient for me to learn it. Would you be willing to do it yourself please?

In DeclarationFormTest::testOralDeclarationRecordedByStaff(), the return statement should be down to just before @todo Fix when added code for cancellation.. It just needs some ajax to add evidence, which might start something like

$this->assertSession()->waitForElementVisible('named', ['button', 'Add new Gift Aid evidence'])->press();

adamps’s picture

Issue summary: View changes
jonathanshaw’s picture

I have never installed the necessary for infrastructure for FunctionalJavascript tests. Probably we need just one test, so it's not really efficient for me to learn it. Would you be willing to do it yourself please?

In DeclarationFormTest::testOralDeclarationRecordedByStaff(), the return statement should be down to just before @todo Fix when added code for cancellation.. It just needs some ajax to add evidence, which might start something like

$this->assertSession()->waitForElementVisible('named', ['button', 'Add new Gift Aid evidence'])->press();

Maybe best to make this a seperate issue? I can do it.

adamps’s picture

Issue summary: View changes

I ended up deleting the last two items:

4) Fix deprecation notices in test results (although some come from Commerce tests?)

CheckoutTest has deprecations, but AFAICS they come from Commerce.

Remaining self deprecation notices (2)

2x: Renderer::renderPlain() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Instead, you should use ::renderInIsolation(). See https://www.drupal.org/node/3407994
    2x in CheckoutTest::testCollectDeclaration from Drupal\Tests\gift_aid_commerce\Functional

Other deprecation notices (12)

  12x: Using the 'timestamp' formatter plugin  without the 'tooltip' and 'time_diff' settings is deprecated in drupal:10.1.0 and is required in drupal:11.0.0. See https://www.drupal.org/node/2993639
    12x in CheckoutTest::testCollectDeclaration from Drupal\Tests\gift_aid_commerce\Functional

5) Move the menus one level down, e.g. put Gift Aid under Services? This is the normal level, and it means that the various admin pages for the module are all visible as tabs.

Things have changed since I wrote that originally. Each of the admin pages now has its own tabs, so it no longer applies.

adamps’s picture

Maybe best to make this a seperate issue? I can do it.

Thanks I raised #3536346: Fix FunctionalJavascript test.

  • adamps committed 80eb01f6 on 1.x
    Issue #3535869 by adamps, jonathanshaw: Tidy up
    
adamps’s picture

Assigned: jonathanshaw » Unassigned
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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