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

QUAL Rename column fk_origin_line on shipment lines into fk_elementdet to match a generic use. #28989

Merged
merged 33 commits into from
Apr 8, 2024

Conversation

zephyriony
Copy link
Contributor

New [Adapting the query and code to the new table format]

Modifying the fetch method

@@ -24,7 +24,7 @@ create table llx_expeditiondet
rowid integer AUTO_INCREMENT PRIMARY KEY,
fk_expedition integer NOT NULL, -- ID of parent object
fk_element integer, -- ID of main source object
fk_origin_line integer, -- ID of line of source object (proposal, sale order). TODO should be renamed into fk_elementdet in SQL files and code in same PR
fk_elementdet integer, -- ID of line of source object (proposal, sale order)
Copy link
Member

@eldy eldy Mar 24, 2024

Choose a reason for hiding this comment

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

If you update the field in table used to create a new instance, you must also add a line into file htdocs/install/mysql/migration/19.0.0-20.0.0.sql to migrate the field name.

Also, it seems the field is still used in a lot of other file with name fk_origin_line, all mention to this file for expeditiondet must modofied in same PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there interractions with the sql database in other files?
Normally, the principle of the model is that you only use it for sql interractions, right? For other files, we use the object properties. That's why, for the moment, I've left the fk_origin_line property, but put the value of the new field in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eldy I must not be very clever, but I can't find anything about the old sql field in the files. Can you give me an example of a file where the old sql field is still present?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @zephyriony
Can you fix conflicts so i can help and push changes to your branch to complete this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sonikf
Yes thank you it would be great I have less time to deal with it now but I think it is an important point for the future version of Dolibarr ^^
I solved the conflict normally :)

@eldy eldy changed the title Adapting the expedition class to its table QUAL Rename column fk_origin_line on shipment lines into fk_elementdet to match a generic use. Apr 8, 2024
@eldy eldy merged commit 6f52440 into Dolibarr:develop Apr 8, 2024
6 of 7 checks passed
@sonikf
Copy link
Contributor

sonikf commented Apr 8, 2024

@eldy
Can you please suggest next step?

@eldy
Copy link
Member

eldy commented Apr 8, 2024

@eldy
Can you please suggest next step?

I think database is ready for standalone shipment/reception. Because we made a lot of change into structure (above all on reception side), and because freeze of v20 would start soon, i think we should not work on screens on shipment/reception anymore, except to fix regression we introduced by renaming. And we should wait next version for that. We already have a very high number of structural changes in develop, so it would be better to make a pause for v20... But i don't rule, coming contributions rules... it's just a point of vue.

@sonikf
Copy link
Contributor

sonikf commented Apr 8, 2024

Ok I understand.
Could you provide steps for future reference though?

eldy added a commit that referenced this pull request Apr 8, 2024
* Fix PR #28989, create shipment line

* Fix Phan

* Update changelog

* Fix stan

---------

Co-authored-by: Laurent Destailleur <[email protected]>
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.

3 participants