Skip to content

[WIP] Fix/cookies jar submit #2668#2707

Draft
Sushanth012 wants to merge 2 commits into
soxoj:mainfrom
Sushanth012:fix/cookies-jar-submit
Draft

[WIP] Fix/cookies jar submit #2668#2707
Sushanth012 wants to merge 2 commits into
soxoj:mainfrom
Sushanth012:fix/cookies-jar-submit

Conversation

@Sushanth012

Copy link
Copy Markdown

Summary

Fixes the cookie TODO in maigret/submit.py (line 218), part of #2668.

Changes

  • check_features_manually now accepts cookie_filename, loads the
    cookie jar, filters cookies for both existing and generated URLs,
    and passes them into the HTTP comparison calls
  • get_html_response_to_compare now forwards optional cookies to
    session.get
  • Added a regression test in tests/test_submit.py confirming the
    cookie jar is loaded and filtered for both URLs

Closes part of #2668

@soxoj soxoj marked this pull request as draft May 28, 2026 08:15
@soxoj

soxoj commented May 28, 2026

Copy link
Copy Markdown
Owner

Hi @Sushanth012, thanks a lot for picking up sub-task 1 from #2668 — the cookie plumbing through check_features_manually looks correct and the regression test is well-scoped. I'd like to land that part 👍 🆒

However, this PR also contains a second, unrelated commit — "Improve graph visualization: dark theme, node info, linking" — which is not mentioned in the title or description and touches ~150 lines in maigret/report.py. Could you please split the PR into two:

  1. Cookies PR — keep only the submit.py + tests/test_submit.py changes from this branch. I'm happy to merge that once it's isolated.

  2. Graph visualization PR — open separately, with before/after screenshots and a short rationale.

Splitting the work makes both changes much easier to review, and lets the cookie fix land immediately while we iterate on the visualization. Thanks again for the contribution! 👍

@soxoj soxoj changed the title Fix/cookies jar submit #2668 [WIP] Fix/cookies jar submit #2668 Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants