Use Gitlab CI in Simplenews

AdamPS → closed merge request !38

Strange - it worked here, and on my PC, but it shows as failing at https://git.drupalcode.org/project/simplenews/-/pipelines

I don't seem to have permission to change settings, in particular set the default branch on gitlab.

It failed here too. The green results are drupalci, the red x is gitlab.

The difference is almost certainly that gitlab ci defaults to the supported core version, which is 10.1.

Note that removing drupalci.yml doesn't disable those tests, you need to do that on the settings page and instead set up pipeline schedules with variables overides

It failed here too. The green results are drupalci, the red x is gitlab.

Ah thanks for explaining

Note that removing drupalci.yml doesn't disable those tests, you need to do that on the settings page and instead set up pipeline schedules with variables overides

Thanks I'll do that once it's all working.

The difference is almost certainly that gitlab ci defaults to the supported core version, which is 10.1.

That was my first idea, but apparently not. D10.1 works on my PC, and on Drupal CI see https://www.drupal.org/pift-ci-job/2792108 → .

3.x works on gitlab, see https://git.drupalcode.org/project/simplenews/-/pipelines.

The failures on 4.x are all related to hash authentication. Could there be something strange on gitlab with timestamps or \Drupal::service('private_key')?

Hm, I see, I think you have weird encoding issue, looks like the hash can contain special characters?

You can download the artifacts, that contains the browser output HTML files, took me a moment to find a relevant one (you want the last of a test run (specific last ID) for that given test class. For example in Drupal_Tests_simplenews_Functional_SimplenewsSubscribeTest-26-22990325.html, you can see this URL:

GET request to: http://localhost/web/simplenews/confirm/1/1697995771/JuMNJEuYo0UoPdbRXK%...

And that's a 404. That "%80%A6" really doesn't look like something that should go in the URL, you might need to ensure that the hash/key you generate are URL safe? I don't know if the actual difference then is the webserver that's used or if it's just random.

Great thanks for the help.

The hash generation code uses Crypt::hashBase64() and apparently hasn't changed since 2014. I think the problem might be in extractConfirmationLink() picking up the wrong link - that function changed a little in 4.x and maybe isn't precise enough now. I don't have any idea why it fails only on gitlab. Let's try like this...

Wrong guess. And I accidentally caused a test error, which happily has revealed the full contents of the mail. It's exactly as expected apart from the corrupted URL.

Interestingly the URL ends with %E2%80%A6 which is UTF-8 for "…" an ellipsis. Something on the gitlab test run is truncating the URL, but doing it in the wrong encoding. However it seems to work on gitlab for 3.x.

I searched Core/simplenews and the main likely ellipsis is in Unicode::truncate(). There are a few places in Core that call that including _filter_url_trim().

I found simplenews has MailFormatHelper::absoluteMailUrls() which looks for "..." - three separate dots, not an ellipsis. So this code is presumably broken because Core uses an ellipsis. But it doesn't seem related to this problem here.

I'm running out of ideas it's very mysterious😃.

I created #3424725: [Meta] Switching to Gitlab CI → and added this issue, to maybe get some assistance?

I'm happy to help here, MR40 has a conflict, so it'd be great to fix that before any further investigation.

Did you know that you can browser or download artifacts generated for the "phpunit" job https://git.drupalcode.org/project/simplenews/-/jobs/216531? See the buttons on the sidebar. It's useful to track which test failed, then browse to that one and see what's on the page. It's very useful for debugging.

One big difference between Drupal CI and GitLab CI is that the former was very forgiving with the links when the site was set up in a subdirectory. GitLab CI is a bit more strict and ensures that links are properly formed (we needed to adapt a few tests in core with this issue). I'm mentioning this because most of the errors are "blablabla was not found anywhere", and that's probably because it ended up in a 404 or something similar.

I'll try to have a look to some of the failing tests and see if I can find something tho.

I went to https://git.drupalcode.org/project/simplenews/-/jobs/216531/artifacts/br...
and looked for Drupal_Tests_simplenews_Functional_SimplenewsPersonalizationFormsTest-1-92075051.html, which is the first test run for that file, the one reporting the error. Then I wait for the page to load (https://project.pages.drupalcode.org/-/simplenews/-/jobs/216531/artifact...) and start clicking next, to see all the steps of the test.

On the failing step, I see https://project.pages.drupalcode.org/-/simplenews/-/jobs/216531/artifact..., which contains a random string, but doesn't seem to be the same as the expected one. I'd probably add some debug after $new_value = $this->randomString(20); to display the contents of the page.

I did so here: https://git.drupalcode.org/project/simplenews/-/merge_requests/40/diffs?..., but I can seem to be able to run pipelines in this MR (I reverted the commit). In any case, I was just trying to show what I'd do at this point. The integration seems very close to complete and I feel that once we fix one of those "not found" errors, the others might follow straight away.

If you want more help from my end please say it.

vlad.dancer → made their first commit to this issue’s fork.

I merged 4.x into MR40 and fixed tests. To address issue with ellipsis reported here #15 I corrected regex to continue looking for a right url. It seems that we have a correct email link, but at the bottom of email:

We have received a request to subscribe xSdv@xzcz.yuk [1] at
https://8080-project-drupalpod-8rrv90s5k2j.ws-eu111.gitpod.io/ [2]. To
confirm please use the link below.

https://8080-project-drupalpod-8rrv90s5k2j.ws-eu111.gitpod.io/simplenew…
[3]


[1] xSdv@xzcz.yuk
[2] https://8080-project-drupalpod-8rrv90s5k2j.ws-eu111.gitpod.io/
[3] 
https://8080-project-drupalpod-8rrv90s5k2j.ws-eu111.gitpod.io/simplenews/confirm/8/1715954779/5AZSMrLlUXg1OAHNKTL4J9uB9qkFeGetnRW8-KqSfdw

Great nice work - please can you make some quick changes for comments I added in the MR then I will commit

Great many thanks

Hey @AdamPS woud you mind to add git attribution into the commit or at least do this next time?

It seems like users are no longer automatically checked under the "Credit & committing" section, after they add a commit in an issue ... the same thing happened to me in many issues contained in https://www.drupal.org/project/leaflet_more_maps/releases/2.2.0 → .

I tried to search, but couldn't find an issue, where it was decided to turn off automatic credits after a commit ... Probably to prevent some users from gaming the system? (Which is a real and annoying problem.)

I wonder how we can make sure that maintainers remember to check for credits when changing issue status to "Fixed"?

Note: I am not fishing for credit in this issue, but @vlad.dancer and @fjgarlin should get some credit for their great contributions here with getting Gitlab CI working. And thanks for that!

Agree on the fixing of the credits, thanks both for the reminder to @AdamPS.
--

It seems like users are no longer automatically checked under the "Credit & committing" section,

It wasn't just one issue. We started turning off the auto-crediting features (mostly due to credit gaming but also to be more in line with the future new system) more than a year ago, in several passes. I can't remember which issues were the ones but I'm sure they'll be in the "drupalorg" queue.

Note that the important part to fill up is the table at the bottom of this issue:

The commit message has not been relevant for credit attribution for several years now. It's all in the above table.
--

I wonder how we can make sure that maintainers remember to check for credits when changing issue status to "Fixed"?

We could create an automated message, but that will actually happen when we are in "GitLab issues" as it is already implemented via a webhook when closing the issue. As most of the effort in the coming months will go there I don't know if creating that automatic message in the current issue queue will be a feature request that will be considered. The issue can definitely be created in the "drupalorg" queue.

Thanks for responding so quickly @fjgarlin. I did consider creating a drupalorg issue, but then didn't do it, since Drupal issues will soon move to Gitlab anyway. So it's probably not worth spending energy on building that feature, and then only use it for a short while.

But really great to hear that crediting is a part of the process of closing an issue on Gitlab.

Sorry I hadn't realised that it wasn't added automatically.

I have added credit now. Does that have any effect or is it too late? If too late, is there anything else I can do?

I do appreciate your contribution and I'm sorry if didn't make the correct actions in the system.

It's not late at all. As long as the issue is "Fixed", folks can edit their attribution (if they want to attribute work to their organization).

From your side, just checking the checkboxes is enough. Thanks a lot.

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