Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update merge_company.py #5979

Merged
merged 7 commits into from
Feb 26, 2025
Merged

Conversation

bau123
Copy link
Contributor

@bau123 bau123 commented Feb 25, 2025

Description of change

Allow for the merge tool to let companies with the transferred_field to be included in company merges

Manually tested scenarios:

  • Company_A (has no merge) merged into Company_B (has no merge)
  • Company_C (has no merge) merged into Company_B (has transferred_from)
  • Company_D (has no merge) merged into Company_E (has no merge)
  • Company_E (has transferred_from) merged into Company_B (has transferred_from)
  • Company_B (has transferred_from) into Company_F (has no merge)

Checklist

  • Has this branch been rebased on top of the current main branch?

    Explanation

    The branch should not be stale or have conflicts at the time reviews are requested.

  • Is the CircleCI build passing?

General points

Other things to check

  • Make sure fixtures/test_data.yaml is maintained when updating models
  • Consider the admin site when making changes to models
  • Use select-/prefetch-related field lists in views and search apps, and update them when fields are added
  • Make sure the README is updated e.g. when adding new environment variables

See docs/CONTRIBUTING.md for more guidelines.

Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.62%. Comparing base (126fc2f) to head (172cf63).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5979      +/-   ##
==========================================
- Coverage   96.63%   96.62%   -0.01%     
==========================================
  Files        1084     1084              
  Lines       25414    25414              
  Branches     1678     1678              
==========================================
- Hits        24559    24557       -2     
- Misses        698      699       +1     
- Partials      157      158       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bau123 bau123 marked this pull request as ready for review February 26, 2025 08:16
@bau123 bau123 requested a review from a team as a code owner February 26, 2025 08:16
Copy link
Contributor

@baarkerlounger baarkerlounger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should definitely be some tests here for the scenarios in the description.

Remove outdated comment

Update test_merge_company.py
@bau123 bau123 force-pushed the TET-992-Update-merge-tool-for-companies-merged branch from 5de69dc to fc2bb36 Compare February 26, 2025 14:12
Copy link
Contributor

@DeanElliott96 DeanElliott96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, think the existing tests and the new ones cover the bases! Just a couple comments to try and prevent flaky tests.

@DeanElliott96 DeanElliott96 self-requested a review February 26, 2025 15:11
@bau123 bau123 merged commit 0eb84ab into main Feb 26, 2025
7 checks passed
@bau123 bau123 deleted the TET-992-Update-merge-tool-for-companies-merged branch February 26, 2025 16:00
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.

4 participants