Roadmap

  • Improve unit test coverage.
  • Add migration templates for Drupal 6 and Drupal 7.
  • Support multiple roles - Done
  • views integration - Done
CommentFileSizeAuthor
#27 interdiff-2883617-26-27.txt1.5 KBmradcliffe
#27 agreement-2883617-test-agreement-handler-27.patch9.53 KBmradcliffe
#26 agreement-2883617-test-agreement-handler-26.patch9.31 KBmradcliffe
#23 interdiff-2883617-21-22.txt7.44 KBmradcliffe
#22 interdiff-2883617-21-22.txt42.66 KBmradcliffe
#22 agreement-2883617-migrate-22.patch28.41 KBmradcliffe
#21 interdiff-2883617-20-21.txt502 bytesmradcliffe
#21 agreement-2883617-migrate-21.patch24.23 KBmradcliffe
#20 interdiff-2883617-18-20.txt3.85 KBmradcliffe
#20 agreement-2883617-migrate-20.patch24.25 KBmradcliffe
#18 interdiff-2883617-15-18.txt16.62 KBmradcliffe
#18 agreement-2883617-migrate-18.patch22.32 KBmradcliffe
#15 interdiff-2883617-14-15.txt1.89 KBmradcliffe
#15 agreement-2883617-migrate-15.patch18.43 KBmradcliffe
#14 agreement-2883617-migrate-14.patch16.96 KBmradcliffe
#11 interdiff-2883617-10-11.txt2.22 KBmradcliffe
#11 agreement-2883617-views-11.patch37.82 KBmradcliffe
#10 interdiff-2883617-8-10.txt5.14 KBmradcliffe
#10 agreement-2883617-views-10.patch36.73 KBmradcliffe
#8 agreement-2883617-views-8.patch35.33 KBmradcliffe
#6 agreement-2883617-drupal8-6.patch150.87 KBmradcliffe
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

akashjain132 created an issue. See original summary.

mradcliffe’s picture

Issue tags: -D8 Port

Thank you for your create the issue and your efforts @akashjain132. I appreciate it. I actually have a work-in-progress 8.x-2.x branch in my private repository that incorporates some of my planned architectural changes in #1852834: Possible to have multiple agreements - role based. I had not gotten around to pushing it as I had started working on it before I became a maintainer.

It will probably take me some time to review your work on github to compare it to what I have already, but I would like to try to incorporate any improvements you have there into my own code (with attribution).

At the moment my priority in my free time is to try to incorporate and deal with the merge conflicts around the following feature and bug requests:

- #1852834: Possible to have multiple agreements - role based
- #2873904: Allow more agreement frequency options
- #2374539: Multiple Roles
- #2879720: avoid Drupal API function calls in global context

I don't want to prematurely push the architecture into an official 8.x-2.x branch yet, but I should probably push my 8.x-2.x-WIP branch upstream here at least.

mradcliffe’s picture

Status: Active » Needs work

Because I don't have version control access on the agreement project I've pushed my 8.x-2.x work into a public github repository, https://github.com/mradcliffe/agreement.

I would probably re-fork from my repository, and then add some pull requests until we can create a functional patch to squash down here.

Roadmap:

- Re-implement functional tests as FunctionalJavascript tests.
- Add migration templates for Drupal 6 and Drupal 7.

mradcliffe’s picture

Issue summary: View changes
mradcliffe’s picture

Issue summary: View changes
Status: Needs work » Needs review

This is functional with tests now on github.

Next step is to get access to create branches on drupal.org so that I can move things back here.

mradcliffe’s picture

Version: 7.x-2.x-dev » 8.x-2.x-dev
Assigned: perennial.sky » mradcliffe
Issue summary: View changes
FileSize
150.87 KB

Okay, 8.x-2.x branch created and here's a patch from GitHub.

- Also implements #2374539: Multiple Roles with new functional tests.
- Functional tests ported to PHPUnit.
- Unit test added.

Still needs views integration.

Status: Needs review » Needs work

The last submitted patch, 6: agreement-2883617-drupal8-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
35.33 KB

Adds views integration back.

Status: Needs review » Needs work

The last submitted patch, 8: agreement-2883617-views-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
36.73 KB
5.14 KB

Fixes views schema so that options are set as a sequence instead of a mapping.

mradcliffe’s picture

Let's get those code standards fixed while I'm at it too.

  • mradcliffe committed 5f79839 on 8.x-2.x
    Issue #2883617 by mradcliffe: Adds back views integration
    
mradcliffe’s picture

Title: [D8] Porting to Drupal 8 » [D8] Porting to Drupal 8 - migrations, test coverage
Issue summary: View changes

Updating issue summary to include recently done work.

mradcliffe’s picture

Here's the start of some migration templates and plugins with a test and fixture that will probably fail.

mradcliffe’s picture

Still not working, but I forgot the destination plugin for agreement table, which I think I'm doing what I'm supposed to be doing but the API documentation is not really clear.

The last submitted patch, 14: agreement-2883617-migrate-14.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 15: agreement-2883617-migrate-15.patch, failed testing. View results

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
22.32 KB
16.62 KB

Fixes the fixture issue and gets it almost there.

- Agreement type migration is working.
- Agreement migration is not working. For some reason even though the agreement destination plugin is firing the import method, nothing is getting inserted into the agreement table. :|

Status: Needs review » Needs work

The last submitted patch, 18: agreement-2883617-migrate-18.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mradcliffe’s picture

Status: Needs work » Needs review
FileSize
24.25 KB
3.85 KB

So destinations into a database table aren't supported without migrate_plus? Ugh, so frustrating that it's not documented anywhere and not really obvious given all the other destination plugins. Guess I need to implement my own thing similar to #2769979: Add migrate destination for sql table.

mradcliffe’s picture

- Removes debug code

- todo: d6 fixture and test to make sure that the optional agreement type gets set where it doesn't exist in d6.

mradcliffe’s picture

Adds drupal 6 migration. I split the agreement migration into d6/d7 versions because I needed to explicitly add a config. option to the plugin to determine the version. Tags don't seem to be able to be used for this without an extraordinary amount of extra code.

I'm not sure if it's worth it to add a migration of the agreement settings in drupal 6 and I think that could be a follow-up.

mradcliffe’s picture

FileSize
7.44 KB

Lost the interdiff locally so recreated using interdiff tools.

  • mradcliffe committed c2ee8a8 on 8.x-2.x
    Issue #2883617 by mradcliffe: Adds migration path for Drupal 6 and...
mradcliffe’s picture

Title: [D8] Porting to Drupal 8 - migrations, test coverage » [D8] Porting to Drupal 8 - improve unit test coverage
Status: Needs review » Active

Updating status and title.

I think that this is good enough for an alpha, but I'll leave this open for expanded unit test coverage since most of the tests are functional.

mradcliffe’s picture

Status: Active » Needs review
FileSize
9.31 KB

Adds unit tests for some of the complex logic inside AgreementHandler class. There are still some coverage gaps on the class, but the main complexity should be tested.

mradcliffe’s picture

Not sure why I needed request time defined, but it probably failed at some point in time when I first started this months ago.

mradcliffe’s picture

Issue summary: View changes
Status: Needs review » Fixed

I'm satisfied. Patch is only adding test coverage so that's the last thing I'm going to do for this issue. Additional coverage can probably be added later as bugs are reported and fixed.

  • mradcliffe committed 449e3c9 on 8.x-2.x
    Issue #2883617 by mradcliffe: Adds unit test coverage for...
mradcliffe’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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