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

fix : correct logic for overlap error #38798

Merged
merged 19 commits into from
Dec 27, 2023

Conversation

VihangT
Copy link
Contributor

@VihangT VihangT commented Dec 16, 2023

fixing overlap error logic with taking care of sequential time job cards in overlap job card list

To understand the scenario I have taken live example which is illustrated below.
Ideally PO-JOB01797 should save their time logs as can be seen below image but instead system is throwing an overlap error with mentioning overlap with PO-JOB00678.
When I run the same code for overlap I found the below mentioned job cards in overlap list.
Technically PO-JOB01797 was operated on different station than other mentioned Job Cards still it showed an error.
Here I am explaining my approach towards this:

Screenshot from 2023-12-16 14-45-55

My approach towards overlapping is

  1. Sort the overlapping list wrt from_time
  2. Create sets of Job card which can be sequential.
  3. if any job card is overlapping with any job card in given set then create new set.
  4. do this till all Job cards are bifurcated in to sets.
  5. Count number of Sets for that overlapping list, if number of sets >= production capacity that means new job card can not be accommodate for this time line => So throw overlap Error.

Please provide enough information so that others can review your pull request:

Explain the details for making this change. What existing problem does the pull request solve?

Screenshots/GIFs

@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Dec 16, 2023
@rohitwaghchaure
Copy link
Collaborator

@VihangT test cases failing

changes as per linters
@VihangT
Copy link
Contributor Author

VihangT commented Dec 25, 2023

@VihangT test cases failing

added 1 change as per linters error

fixing overlap error logic with taking care of sequential time job cards in overlap job card list

Added Provision if time_logs list is empty
@VihangT
Copy link
Contributor Author

VihangT commented Dec 26, 2023

Added 1 change after failing in Python Unit Test (2) where time_logs is empty and has_overlap() is called.

@VihangT
Copy link
Contributor Author

VihangT commented Dec 26, 2023

@s-aga-r
Server (Mariadb) / Python Unit Tests (1) (pull_request) and Server (Mariadb) / Python Unit Tests (2) (pull_request) are failing due to showing Error in File "/home/runner/frappe-bench/apps/erpnext/erpnext/accounts/doctype/payment_reconciliation/test_payment_reconciliation.py.

I did not change anything related to that.
@rohitwaghchaure
Should I need to take any action?
Please let me know if any change is required.

@rohitwaghchaure rohitwaghchaure merged commit 92d61eb into frappe:version-14-hotfix Dec 27, 2023
8 of 10 checks passed
@frappe-pr-bot
Copy link
Collaborator

🎉 This PR is included in version 14.58.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-tests This PR needs automated unit-tests. released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants