Problem/Motivation

To allow easy migration of users of SMS Framework to D8, migrations are needed for the sms_user phone numbers verification system to the new D8 phone number verifications system.

Resolution

Develop data migrations for D6 -> D8 and D7 -> D8.

CommentFileSizeAuthor
#17 2858177-17.patch37.44 KBalmaudoh
#11 interdiff.txt5.01 KBalmaudoh
#11 2858177-11.patch37.4 KBalmaudoh
#8 2858177-8.patch37.43 KBalmaudoh
#8 interdiff.txt10.45 KBalmaudoh
#6 interdiff.txt33.16 KBalmaudoh
#6 2858177-6.patch35.89 KBalmaudoh
#4 interdiff.txt2.72 KBalmaudoh
#4 2858177-4.patch7.97 KBalmaudoh
#2 2858177-2.patch7.07 KBalmaudoh

Comments

almaudoh created an issue. See original summary.

almaudoh’s picture

almaudoh commented
Status: Active » Needs review
StatusFileSize
new 2858177-2.patch7.07 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2858177-2.patch, failed testing.

almaudoh’s picture

almaudoh commented
Status: Needs work » Needs review
StatusFileSize
new 2858177-4.patch7.97 KB
new interdiff.txt2.72 KB

Added a rollback for the change to user telephone field. This should pass now in 8.3 since #2851864: Update installation tests is in.

almaudoh’s picture

almaudoh commented
Issue tags: +Needs tests

This needs tests for the migration before it can be committed.

almaudoh’s picture

almaudoh commented
Issue tags: -Needs tests
StatusFileSize
new 2858177-6.patch35.89 KB
new interdiff.txt33.16 KB

Added tests and fixed a whole lot of stuff. PR also on Github

Status: Needs review » Needs work

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

almaudoh’s picture

almaudoh commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt10.45 KB
new 2858177-8.patch37.43 KB

Fixed @group annotation and code standards.

Status: Needs review » Needs work

The last submitted patch, 8: 2858177-8.patch, failed testing. View results

almaudoh’s picture

almaudoh commented

It is weird that in PHP 7.1, the order in which the missing migrations for the D7 migration exception message changes BUT IT DOESN'T AFFECT THE D6 MIGRATION EXCEPTION MESSAGE

I haven't been able to figure out this one...:(

almaudoh’s picture

almaudoh commented
Status: Needs work » Needs review
StatusFileSize
new 2858177-11.patch37.4 KB
new interdiff.txt5.01 KB

Found a way to fix the test using regular expression assertions. Still can't figure out the cause of the weird PHP 7.1 behavior, but it doesn't matter now.

Status: Needs review » Needs work

The last submitted patch, 11: 2858177-11.patch, failed testing. View results

almaudoh’s picture

almaudoh commented
Status: Needs work » Reviewed & tested by the community

All the tests pass except on D8.5 due to newly introduced test fails from use of deprecated API's. See #2870194: Ensure that process-isolated tests can use Symfony's PHPunit bridge to catch usages of deprecated code. This will be fixed in a different issue.

Will commit this later in the day.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Reviewed & tested by the community » Needs work

Id prefer to get HEAD green on 8.5 first, I spotted at least one other error in there. #2922871: Fix test failures in D8.5 due to use of deprecated APIs

almaudoh’s picture

almaudoh commented
Status: Needs work » Reviewed & tested by the community

Id prefer to get HEAD green on 8.5 first, I spotted at least one other error in there. #2922871: Fix test failures in D8.5 due to use of deprecated APIs

There is just one error in there which still only exists in D8.5. I have raised #2925655: Fix failure of SmsFrameworkViewsTest in D8.5 to deal with that.
#2922871: Fix test failures in D8.5 due to use of deprecated APIs reduced the fails in D8.5 to 14, while #2924436: Convert all SmsFramework WebTests to BrowserTests has reduced the errors to zero on D8.4 and one on D8.5 (#2925655: Fix failure of SmsFrameworkViewsTest in D8.5 already mentioned).
Setting back to RTBC.

almaudoh’s picture

almaudoh commented
Status: Reviewed & tested by the community » Needs work

NW for @group legacy in the tests.

almaudoh’s picture

almaudoh commented
Status: Needs work » Needs review
StatusFileSize
new 2858177-17.patch37.44 KB

Added @group legacy to the new tests. This should be ready now.

PR is at https://github.com/almaudoh/smsframework/pull/6/files

almaudoh’s picture

almaudoh commented
Status: Needs review » Reviewed & tested by the community

This issue is blocked by #2924436: Convert all SmsFramework WebTests to BrowserTests and #2925655: Fix failure of SmsFrameworkViewsTest in D8.5. Setting back to RTBC. Will commit immediately those two are committed and we can confirm tests passing on D8.5.

almaudoh’s picture

almaudoh commented
Status: Reviewed & tested by the community » Fixed

Considering that there is only one test fail left in D8.5 head due to #2925655: Fix failure of SmsFrameworkViewsTest in D8.5 and this issue has nothing to do with it, and considering that that issue appears to have become slightly more complicated, I'll go ahead and commit and push this to 8.x-1.x

Let's continue work at #2925655: Fix failure of SmsFrameworkViewsTest in D8.5.

  • almaudoh committed c9a5130 on 8.x-1.x 
    Issue #2858177 by almaudoh: Create data migrations for sms_user to phone...

Status: Fixed » Closed (fixed)

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