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

[MOD] mrp_operations_extension #162

Closed
wants to merge 3 commits into from

Conversation

esthermm
Copy link

There is an issue about adding an element to the workcenter lines, there is an onchange that fills the fields with the values, but as they are readonly when you save the changes of those fields are not saved. So custom_data field is not necessary.

@esthermm esthermm force-pushed the 8.0-mrp_operations_extension branch 3 times, most recently from 1c44173 to a652921 Compare November 2, 2016 13:07
@pedrobaeza
Copy link
Member

👎 You shouldn't remove this option. They are not readonly when you mark custom_data, so no issue at all.

@anajuaristi
Copy link

Could be it's not a technical issue but functional issue.
In my opinion check is not necesary at all. It makes it more complex to
understand the concept and at the end the valid data can be always taken
from routing step.

Check is making necesary more code to control and it's not improving
anything.
This change was asked by one of hour customers and i aproved it
functionally.
Please don't stop the merge.

Manufacturing experts... could you give you opinion on this?
@jordieficent

El 11 nov. 2016 0:42, "Pedro M. Baeza" notifications@github.com escribió:

👎 You shouldn't remove this option. They are not readonly when you mark
custom_data, so no issue at all.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#162 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AALY31vICoxhMRaAwqqTvPKH7HGW9whgks5q86vwgaJpZM4KjMh8
.

@pedrobaeza
Copy link
Member

@anajuaristi, the only to control if the route has specific data is through that check, and you were one of the defenders of that option for your use cases, so I don't understand why you change your mind now.

@anajuaristi
Copy link

No. I wasn't
I always thougth it was no necesary but you included yourself so I let it stay.
But now I think it's causing usability problems to my customers and it's posible to simplify code so I said Esther to erase it.

@anajuaristi
Copy link

anajuaristi commented Nov 13, 2016

Thinking about this, maybe make check active by default (so editable line ) would be enough
I mean... it's much more usual taking cost data from routing line than from machine so just active by default this option but still we would have the option to take from machine.

This way we improve usability withouth loosing functionality.

@pedrobaeza
Copy link
Member

OK, change the default and repair Travis.

@anajuaristi
Copy link

Ok @pedrobaeza
@esthermm please... change the default to set check true and repair travis

@pedrobaeza
Copy link
Member

Thanks for the changes. Travis is still red I'm afraid.

@esthermm
Copy link
Author

Yes, I know, mrp_production_estimated_cost test is failing. I still don't know why

@LoisRForgeFlow
Copy link
Contributor

@esthermm Is this still relevant? are you going to finish the work? Thanks.

@anajuaristi
Copy link

@lreficent it's solved. I'm closing.
Thank you

@anajuaristi anajuaristi closed this Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants