Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.77%. Comparing base (659c648) to head (64e017d).
⚠️ Report is 11 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3381      +/-   ##
==========================================
+ Coverage   92.16%   95.77%   +3.61%     
==========================================
  Files         100      100              
  Lines        6125     6152      +27     
==========================================
+ Hits         5645     5892     +247     
+ Misses        480      260     -220     

@JohnVillalovos kindly have a look for same set of changes with earlier MR rebased on latest. thanks.

@JohnVillalovos appreciate the review, kindly check if it looks good now after review changes. any more suggestions also I would be happy to rectify sooner. thank you.

@JohnVillalovos appreciate the review, kindly check if it looks good now after review changes. any more suggestions also I would be happy to rectify sooner. thank you.

Thanks. I will try to find time and motivation to review this. Hopefully this week.

As a note. It would be nice if it was squashed down to maybe 1 commit? Less than 12 sounds like a good start.

@isaac-philip Now it is 13 commits. Let me know if you need help squashing

Hello @JohnVillalovos , have made the change as requested for single commit, thanks.

@isaac-philip Please make the commit message a bit more informative

Currently it is basically just a subject line that says:

feat(functional): merge-train api add and status #2547

It is hard to tell from the message what is being added, what “status” refers to, or whether this is about merge train objects, merge train merge requests, or functional test coverage.

Also the PR number should not be in the commit message.

I was reviewing this with the help of Claude Code. I ended up coming up with this.

The nesting of ProjectMergeTrainMergeRequestManager under ProjectMergeTrain is misleading because the merge train's ID is never used in the URL. Compare with the established pattern in this codebase:

# ProjectDeploymentMergeRequest — deployment id IS in the path:
_path = "/projects/{project_id}/deployments/{deployment_id}/merge_requests"
_from_parent_attrs = {"deployment_id": "id", "project_id": "project_id"}

# ProjectMergeRequestDiffVersion — MR iid IS in the path:
_path = "/projects/{project_id}/merge_requests/{mr_iid}/versions"
_from_parent_attrs = {"project_id": "project_id", "mr_iid": "iid"}

In both cases _from_parent_attrs includes the parent's own ID so it genuinely scopes the child resource. Here, _from_parent_attrs = {"project_id": "project_id"} only borrows project_id — the merge train's id is silently discarded. This means:

project.merge_trains.get(1,   lazy=True).merge_requests.get(5)  # GET /projects/1/merge_trains/merge_requests/5
project.merge_trains.get(999, lazy=True).merge_requests.get(5)  # GET /projects/1/merge_trains/merge_requests/5  (identical)

The 1 vs 999 makes no difference. A user would reasonably expect the merge train ID to matter.

The GitLab API endpoint /projects/:id/merge_trains/merge_requests/:iid is a project-level resource, not scoped to a specific merge train. The consistent approach would be to attach the manager directly to Project, alongside merge_trains:

# projects.py
merge_trains: ProjectMergeTrainManager
merge_train_merge_requests: ProjectMergeTrainMergeRequestManager

# merge_trains.py
class ProjectMergeTrainMergeRequestManager(...):
    _path = "/projects/{project_id}/merge_trains/merge_requests"
    _from_parent_attrs = {"project_id": "id"}  # "id" from Project directly

Usage becomes project.merge_train_merge_requests.get(mr_iid) — accurate and consistent with the rest of the codebase.

What do you think @isaac-philip ?