This issue is part of meta issue #1931088: [META] Fixing tests

I have enabled the location_phone module but unable to make it required. After doing some debugging I found on in location_phone.module file at line 29 in location_phone_locationapi function it should be
'#required' => ($a4['collect'] == 2),
instead of
'#required' => ($a4 == 2),

CommentFileSizeAuthor
#9 location-phone_field_required-1646400-9.patch1.12 KBtucho
#1 phone_field_required-1646400-1.patch576 bytesjibran

Comments

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Active » Needs review
StatusFileSize
new phone_field_required-1646400-1.patch576 bytes

Here is the patch.

bertramakers’s picture

bertramakers commented

I applied the patch from comment #1 and this worked for me.

jyee’s picture

jyee commented
Version: 7.x-1.0-beta1 » 7.x-3.x-dev
Status: Needs review » Reviewed & tested by the community

Confirmed that patch #1 applies and works on both the current 7.x-3.x branch (revision cb2bb35) and also to the 7.x-3.0-alpha1 release.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Reviewed & tested by the community » Postponed

postponed before tests fix
#1931088: [META] Fixing tests

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Postponed » Needs review

#1931088-3: [META] Fixing tests mostly fixed for 7.x-3.x branch
this one needs review

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Title: Unable to make phone field required. » [Needs tests]Unable to make phone field required.
Status: Needs review » Needs work
Issue tags: +Needs tests

#1 commited pushed to 7.x-3.x
thanks!

can You provide test coverage for this?

raulmuroc’s picture

raulmuroc commented
Status: Needs work » Fixed

This has been several times tested and the patch passes the drupal's test. Should be marked as fixed, should not?

jyee’s picture

jyee commented
Status: Fixed » Needs work

No, it should not be marked as fixed. Even though podarok has commited the patch, he is still requesting that simpletests (not user reviews) be written to specifically test this functionality.

jyee’s picture

jyee commented
Issue summary: View changes

Updated issue summary.

tucho’s picture

tucho
Primary language Spanish
Location Buenos Aires
commented
Issue summary: View changes
StatusFileSize
new location-phone_field_required-1646400-9.patch1.12 KB

If the hook_locationapi was invoked with a non-array $a4, it will throw a warning.

I added the same solution applied in location_locationapi and I made a new patch starting from the actual dev version.