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 #26920 use status instead of fk_statut #27929

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

evarisk-micka
Copy link
Member

@evarisk-micka evarisk-micka commented Jan 31, 2024

FIX #26920
Update SQL task field fk_statut in status
First pull request concerning issue #26920 , others will follow to complete the work (but better go step by step)

@hregis hregis closed this Jan 31, 2024
@evarisk-micka evarisk-micka reopened this Apr 5, 2024
@evarisk-micka
Copy link
Member Author

@eldy can you give a review please ?
There is an inconsistency at this level and we need the status field in contract (this pr was merged #27930 but not this one)

@eldy eldy added PR to fix or conflict to solve PR needs to be fixed to be integrated (except for conflicts, a comment describes the fix to do) PR to fix - Continuous Integration Test failed The PHP unit tests return something wrong with our code. Check the details to know what to fix. labels Apr 26, 2024
@eldy
Copy link
Member

eldy commented Apr 26, 2024

Duplicating fields is never a good idea. It means we will get data in conflict.

For a transition to rename a field we must first :

Step 1 - Manage in php code the both property field ->fk_status and ->status and set old one as deprecated.
So existing modules are compatible and can be updated using the new field name being also compatible.
We can also use alias a "as status" into sql to have directly the new name as result of sql $obj->status

Step 2 - when we are sure that step 1 is completed (we can use ->oldname and ->newname in code, for both create,update and read, with the same result), then in a next version n+x, we update field in database with the new name and update sql requests in core code.

Step 3 - Then in a next version n+y, we remove the duplicated code related to fk_status.

Currentlty, there is still work to do on step 1

@eldy eldy added PR not qualified PR is not qualified (feature not enough requested, duplicate feature or other reason) and removed PR to fix or conflict to solve PR needs to be fixed to be integrated (except for conflicts, a comment describes the fix to do) PR to fix - Continuous Integration Test failed The PHP unit tests return something wrong with our code. Check the details to know what to fix. labels Apr 26, 2024
@aspangaro aspangaro closed this Aug 18, 2024
@lmag
Copy link
Member

lmag commented Aug 20, 2024

I update #26920 we check tomorrow step 1 and 2

@lmag lmag reopened this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR not qualified PR is not qualified (feature not enough requested, duplicate feature or other reason)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow for project and task
6 participants