fjgarlin → made their first commit to this issue’s fork.
Doing @gitlab_username is doable as we have that property stored in the user object. That'd be a relatively easy change.
For the d11-recipe branch, if we want to start with no tests, that'd be ok as well, we can add them at a later stage, as what we are doing now is the kind of first pass at supporting recipes.
Great work! MR is merged.
I found an issue tho, but didn't want to block the whole MR for this.
See these images:
vs
So, for the drop-pattern-dark-blue background, the image borders are not visible. This can be addressed in a new MR.
You wouldn't get an issue notification if the credits are updated, the issue lives in D7, the credits in D10, and they won't trigger a notification just for saving them. You got the notification because there were actual changes in the issue made in October 5th by Benji.
The users are notified when they are added via the "credit others" field, and not in any other scenario. In this case, you created the issue, and you were added automatically to the contribution record, but no notification is expected.
I think all feedback is addressed now.
There is an overall WIP to make many of the UI elements ajax-driven, and that includes polling for branches too.
This issue would belong to the "core" queue.
But I'm not sure how useful this can be. If you look for " https://www.drupal.org/node/ → " in core's codebase you can find thousands of references to issues. On 11.x:
$ grep -r 'https://www.drupal.org/node/' . | wc -l
2142
Some of those are references to issues, some of them to documentation pages, some of them to change records. Some of the references to closed issues are even intentional.
Add user as member at the same time that we are creating the fork: https://git.drupalcode.org/project/drupalorg/-/commit/a38e08d6cf7864cd5c...
Feedback addressed:
- DrupalOrgIssueForksQueueWorker::getAccessLevel() to be made a constant: https://git.drupalcode.org/project/drupalorg/-/commit/29790d172e9c5d330f...
Delaying logic: https://git.drupalcode.org/project/drupalorg/-/commit/25feba6992648d1dfb...
I created a second queue.
- We re-queue in the initial queue up to 20 times, after that we send the item to the "delayed queue"
- We check the delayed queue every 30 seconds. If the item was queued less than 30 seconds, ignore it, otherwise send it back to the original queue. This process has a max attempts limit as well.
I went for this instead of the sleep as requests could potentially queue and a bad/stuck one could affect others. I did not do a second rabbitmq queue because then we cannot use the delay exception implemented in the database queue.
The "delayed queue" is a generic one, so we can use it for any other queues where we need similar logic.
About doing more actions right after the fork creation, than can delay the feedback to the user. We can experiment with it once we have changed the logic to be more JS driven (big WIP).
Good feature request and great/simple implementation. Looks good!
RTBC.
Most of the logic is here:
- https://git.drupalcode.org/project/drupalorg/-/commit/1371edcd33192378fd...
Custom tracker is on the D7 script to migrate issues:
- https://git.drupalcode.org/project/drupalorg/-/merge_requests/105/diffs?...
I think this covers everything. Can you review @drumm?
MR with the change needs review: https://git.drupalcode.org/project/drupalorg/-/merge_requests/409
fjgarlin → changed the visibility of the branch 7.x-3.x to hidden.
Merged.
Will update ✨ Replace credit URLs with modern Drupal ones Active to use this filter.
📌 Use relative time ranges for filtering credit listings Active adds the option to use "months" as parameter, rather than the fixed timestamp. This will make linking issue listings for "last X months" always work.
I'll create an MR to adapt the links we added to use that parameter instead.
This is a good feature. I should have added it from the beginning since it is controller based.
I suggest we call the param months, make it "int" and it will just be "months ago".
There was a small bug that was undercounting results for the aggregated block on the modern Drupal side of things. Discovered by @pfey (already credited). The views were ok as they already had a similar fix.
Fix: https://git.drupalcode.org/project/contribution_records/-/commit/adcfee8...
It's probably inevitable that some names become obsolete. I'd go with d11-recipe for this one.
It's like all the old module naming convention 8.x-1.x... which is still valid for D11.
I added some minor feedback in the MR. Things are looking really good.
Ready for review. It will render like this:
MR: https://git.drupalcode.org/project/drupalorg/-/merge_requests/405
I wouldn't sed the string for valid characters. All we need here is just to trim both ends of the string of any spaces/line break characters.
I'd rather have a job to fail and break because the maintainer typed mod ule than pass and make the maintainer (or contributor) think that it's ok.
My idea here is really just detect all *.info.yml files, extract the core_version_requirement and if it's different, report it.
We don't need to do much I think, just a warning to make maintainers aware of this situation.
and wait for autocomplete
No need to wait, paste the three usernames (comma separated) and submit. Drupal should take care of the matching if I remember well.
The pipelines are happy with the change and it now tests D10 (with PHP8.1).
The issue we were having, in latest stable D10 and PHP8.1 was:
ParseError: syntax error, unexpected token "readonly", expecting "abstract" or "final" or "class" in Composer\Autoload\{closure}() (line 21 of /app/web/modules/contrib/mailchimp/src/ApiService.php).
final readonly class ApiService
That is a PHP8.2 only feature and it breaks Drupal sites running on PHP8.1. The PHP constraint could be indicated in composer.json to avoid this.
Doesn’t the credit others input cover that partially? You can paste there the list of usernames and save. You could have that list as part of the issue template so is quicker to copy.
https://docs.gitlab.com/api/project_integrations/#disable-a-custom-issue...
Needs to be on the D7 migration. We need to reset it so it's GitLab's one.
As for the rest. 100% correct. I've added links and todos to the code and I will work on it.
I had a few changes in progress and I added this as part of those. See the commit https://git.drupalcode.org/project/drupalorg/-/commit/a2dfaae63f1933b6cb.... The trait function is gone and all usages were changed to the user service function.
Good find. The "getGitLabUserIdFromUserId" predated that new migration and function.
I think i actually did this today. Here: https://git.drupalcode.org/project/drupalorg/-/commit/ac77f0cb13ffc4926f...
It follows the current name pattern we already have.
Core issue queue. That's where it is controlled. You can use the component "phpunit" and mention that is also useful/needed for the contrib integration.
Added link to contribution records and changed formatting of the top of the issue description with the extra info.
The values brought to the contribution record after adding the above users to it are correct and match exactly what's on the defaults.
@avpaderno has: volunteer = 1, attribute orgs = 0
@joachim has: volunteer = 0, attribute orgs = 1 (+ the attribution)
Closing this issue as it was just a test.
The values brought to the above issue match exactly what's on the defaults.
@avpaderno has: volunteer = 1, attribute orgs = 0
@joachim has: volunteer = 0, attribute orgs = 1 (+ the attribution)
Testing if the saved defaults are brought in a brand new issue: #3548121: [ignore] Test issue →
@avpaderno
$values = \Drupal::service('user.data')->get('contribution_records', 55077, 'user_contribution_defaults');
= [
"volunteer" => [
"value" => 1,
],
"attribute_orgs" => [
"value" => 0,
],
"organizations" => [],
"customers" => [
[
"target_id" => null,
"_weight" => "0",
"_actions" => [],
],
],
]
@joachim
$values = \Drupal::service('user.data')->get('contribution_records', 107701, 'user_contribution_defaults');
= [
"volunteer" => [
"value" => 0,
],
"attribute_orgs" => [
"value" => 1,
],
"organizations" => [
[
"target_id" => "3500944",
],
],
"customers" => [
[
"target_id" => null,
"_weight" => "0",
"_actions" => [],
],
],
]
The second checkbox wasn't so prominent in the previous system, so it might be that you had it ticked but never realised/cared about it. If there are no organizations linked it doesn't really matter anyway.
What's shown when you are looking at it is the values for that record, always. The "defaults" are not seem until they are used for a new attribution, and in that moment, they become the value for that record.
Re #22 - what you see is always what is saved already. It’s always an edit form for the existing attribution.
A link to either the issue or the contribution record is really all we need. From there I will look first thing at the date of the issue or the comment.
If the issue/comment happened before August 29th, then the attribution that was set in the previous system for that issue is kept on the contribution record. We had integrity checks that checked that all the information was a match in the old vs new system.
If the issue/comment happened after that date (and the user was not yet part of that issue/contribution record), then it depends on whether the user logged in to the new system and set defaults. If they didn't log in, the new system wouldn't make any assumptions (the old system wasn't keeping "defaults", it was only remembering the last used values). If they logged in and saved values as default, then those will be used from that moment on. So there is a period between August 29th and when the user saved defaults where things might not be as expected. Note that clicking "Save" only saves information for _that_ record, cliking "Save and set as defaults" saves information for that record _and_ future attributions.
Once you save default values in the new system, those are used every time that your user is brought into a contribution record (either by making a comment in an issue or if you are credited by maintainers using the "Credit others" field).
107701 is your user ID.
> $values = \Drupal::service('user.data')->get('contribution_records', 107701, 'user_contribution_defaults');
= [
"volunteer" => [
"value" => 0,
],
"attribute_orgs" => [
"value" => 1,
],
"organizations" => [
[
"target_id" => "3500944",
],
],
"customers" => [
],
]
Only line in the code that reads that value: https://git.drupalcode.org/project/contribution_records/-/blob/1.0.x/con...
$values['volunteer'] = $values['volunteer']['value'] ?? 0;
Line of code that uses the above:
https://git.drupalcode.org/project/contribution_records/-/blob/1.0.x/con...
$paragraph->set('field_contributor_volunteer', (int) $values['volunteer']);
I can't see anything wrong with the code. Maybe you do. I haven't had any other reports of this not functioning so I'm a bit puzzled as I can't replicate it.
Code looks good to me. It provides additional info in the key places. Thanks for the improvements!
They might need to address it sooner or later, but yeah, an issue on their queue seems like the best option.
RTBC for this MR for what is worth.
Agree. Maybe we can open a follow-up to either re-add it or try another module.
All the documentation lives in the same repo, in the “docs” folder.
I think so. You found the way to make it work, so all we need to do for now is to document it.
Are you sure? As part of making the comment above, you were brought to the contribution record associated to this issue here https://new.drupal.org/contribution-record/11423491, and I can see that your entry is NOT marked as volunteer, indicating that it's using your user defaults.
If you give us reproducible steps we can look into it, but it all appears to be working as expected for your user, given the above.
We can even use PHP and include the file conditionally depending on the Drupal version.
See an example of this here: https://git.drupalcode.org/project/project_browser/-/blob/2.0.x/phpstan.... (and then see the PHP file).
I like the second suggestion better.
Or the first one only if we don’t end up using “before_script”. But this code manipulation seems risky.
Notes for myself (or anybody wanting to help):
- Add method "getCategory" to "SourceLinkInterface.php"
- Implemente this method in:
--- DrupalOrgIssue.php: where we read the "field_issue_category" field and map the 5 categories to some agreed values.
--- GitDrupalCodeBase.php: where we read the issue labels, filter "category::*" ones and do some kind of mapping too (or just take what's after the "::"). Note that there could be more than one in this scenario so a decision should be made as for what to take.
Then we could use this method to pass information to the page and set the input default value.
I'm happy for people to work on it; it doesn't need to be postponed.
There are 11 proposed values for the value of that input in here ✨ Usefull help message or clickable fill for type? Active , some of them match the values from the "Category" on Drupal.org and some of them don't.
Also, issues are moving to GitLab issues within the coming weeks, and that will be just labels on the new system, so any work done here will need to be rewritten based on that.
RTBC. Not sure either about the new deprecations but the changes make sense.
I don't understand it either. All we can hope for is just to have the output there and to show the before and after. What the actual tool does is out of our control I think.
The code looks good in my opinion. Not sure if you are planning on adding/removing any more debug lines.
So far, that input is static, pending decissions on ✨ Simplify agreed commit format Active .
We'll need to evaluate this. It feels like it can be achieved with a view. This view could be at user level, project level or even global with filters and conditions (eg: filter by project).
This is not something for this module per-se, it's more of a Drupal.org feature request, so I'm moving it to that queue. It will need to be in the new site.
Yeah, the plan is:
- Keep d.o NID as gitlab issue IID, losing author metadata information (it will be in the description)
- Do not keep/do comment redirects (severe performance implications)
- Post migration, gitlab issues from separate projects could have the same IID.
- If you move an issue, gitlab creates/keeps redirects for that
Note that if you have email notifications set up for your projects issues, you get notified when the issue is closed with the same comment that is posted on the issue, reminding you to attribute credits.
I don’t know how much different this suggested approach can be.
Re redirects, that has been the case for a long time. There are two existing redirects per issue right now.
We are adding this third redirect in the code:
_drupalorg_gitlab_add_redirect('node/' . $record->nid, $issue_gitlab_url);
So, when this is done:
- node/NID will 301 redirect to the gitlab issue
- i/NID will 302 redirect to the d.o issue, which will 301 redirect to the gitlab issue
- project/PROJECTNAME/issues/NID will 301 redirect to the gitlab issue
In order to run an earlier version of phpunit we could add an entry in the "composer.json" file for "phpunit/phpunit" with the desired version.
For now, giving priority to keeping IID over issue author, I've added the author information in the description of the migrated issue: https://git.drupalcode.org/project/drupalorg/-/merge_requests/105/diffs?...
It'd show like this:
Re preserving NID so it becomes the IID of the new issue (requested in several comments).
This is possible if the issue is created as admin. However:
- Doing this makes the author of the issue the admin user.
- Impersonating the user when creating the issue makes the author correct, but does NOT preserve IID.
There is no api method to change the author of an issue. It needs to be set correctly on issue creation.