- ๐ฎ๐ชIreland lostcarpark
Apologies, but I should have thought of this in the original issue description.
I think test coverage is needed to ensure the validation is working.
In the
VerifyEmailSetupTest::testVerifyEmailConfig
, the test assumes that the save of the verify email form was successful. However, this is no longer a valid assumption since it could fail validation.- After the first submit on line 65, we should use
addressEquals
to verify the form saved and returned to the/admin/people/verify-email
page. - We should also
statusMessageContains
to check the status message is correct. - We can remove the
drupalGet
on line 66 since the page should have redirected there anyway.
We should also add a new test function in this class to test that an invalid destination path won't save.
- This should attempt to save a new verify email form, but specifically put a path that doesn't exist in the destination path
- It should assert with
addressEquals
remains on the add page. - It should assert with
statusMessageContains
that the correct error message is generated.
Thank you for your work on this.
- After the first submit on line 65, we should use
- Issue created by @ifrik
- @peterlemmens opened merge request.
- First commit to issue fork.
- @peterlemmens opened merge request.
- First commit to issue fork.
- @robbesaelens opened merge request.
- @eli-t opened merge request.
- Issue created by @Eli-T
- First commit to issue fork.
- Issue created by @dtfabio
- @robbesaelens opened merge request.
- Issue created by @dtfabio
- ๐ฎ๐ณIndia nidhi27
Hi,
I have tested the functionality and its working as expected. But test cases are failing because somewhere path is added. We can change it to any valid internal url. (E.g. )
Moving it to needs work.
- ๐ง๐ชBelgium robbesaelens Kortrijk
robbesaelens โ changed the visibility of the branch 3519847-support-oop-hooks to hidden.
- First commit to issue fork.
- @robbesaelens opened merge request.
- ๐ง๐ชBelgium robbesaelens Kortrijk
robbesaelens โ changed the visibility of the branch 3519846-support-oop-hooks to active.
- ๐ง๐ชBelgium robbesaelens Kortrijk
robbesaelens โ changed the visibility of the branch 3519846-support-oop-hooks to hidden.
- ๐ง๐ชBelgium tim-diels Belgium ๐ง๐ช
Can be tested once ๐ Add GitLab CI configuration Active is merged so pipelines are activated, but already provided the fixes.
- @tim-diels opened merge request.
- ๐ง๐ชBelgium tim-diels Belgium ๐ง๐ช
Created 2 subtasks. One for CSpell and one for PHPCS. PHPStan will solve itself when the Drupal 11 compatibility is merged.
- Issue created by @tim-diels
- @jurgenhaas opened merge request.
- @tim-diels opened merge request.
- Issue created by @tim-diels
- ๐ซ๐ทFrance luukyb
Merge request is now ready for review, I will assign to you @nx2611.
Screenshot attached.
Thanks! - @luukyb opened merge request.
-
tim-diels โ
committed ebf46fc0 on 1.0.x
Issue #3519840 by tim-diels: Stricter PHP quality tests in GitLab CI
-
tim-diels โ
committed ebf46fc0 on 1.0.x
- Issue created by @luukyb
- ๐ฌ๐งUnited Kingdom nlisgo
This has been resolved in https://www.drupal.org/node/3371753 โ
Although this issue is older than the above I am marking this as the duplicate because the above has been marked "Closed (fixed)".
- First commit to issue fork.
- Issue created by @svendecabooter
- Issue created by @svendecabooter
- Issue created by @svendecabooter
- Issue created by @svendecabooter
- Issue created by @svendecabooter
- @tim-diels opened merge request.
- Issue created by @svendecabooter
- Issue created by @svendecabooter
- Issue created by @tim-diels
- @ankit_kumar opened merge request.
- Issue created by @lostcarpark
- @alexortega_98 opened merge request.
- Issue created by @a.dmitriiev
- ๐บ๐ธUnited States smustgrave
Alright whatever floats your boat doesnโt bother me
- Issue created by @valthebald
- ๐บ๐ธUnited States kmonty San Francisco, CA
I mean you could have clicked on my profile to see I've been in the community for 17+ years and maintained multiple modules and a profile. But yeah I'm just a spammer in your eyes.
- ๐บ๐ธUnited States smustgrave
Donโt pause at all. Just would aim for other tickets.
If youโre new to contribution there is a novice tag that people tag for good entry level stuff.
Also contribution comes in different forms. You were looking at that soft limit ticket. You can test it out and see if it works, posting your steps, screenshots if relevant, etc. Thatโs credit worthy too
- ๐บ๐ธUnited States kmonty San Francisco, CA
Apologies, I was just trying to get โจ Add a Soft Limit option Active to pass.
I'll pause contributing to not waste your time.
- ๐บ๐ธUnited States smustgrave
So I would make a recommendation not to open phpcs tickets, unless maybe the maintainer does. Many maintainers like myself have been spammed endlessly with these to the point they got a negative outlook. If interested in this module though we got plenty of open bugs that need help
- ๐บ๐ธUnited States kmonty San Francisco, CA
Noting that merging this will get โจ Add a Soft Limit option Active passing.
- @kmonty opened merge request.
- Issue created by @kmonty
- Issue created by @yannickoo
- Issue created by @yannickoo
- Issue created by @yannickoo