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

[16.0][MIG] project_sequence: Migration to version 16.0 #1155

Merged
merged 9 commits into from
Jul 26, 2023

Conversation

NICO-SOLUTIONS
Copy link
Member

@NICO-SOLUTIONS NICO-SOLUTIONS commented Jul 15, 2023

standard migration

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 16.0-mig-project_sequence branch 13 times, most recently from 91f9466 to d106bb4 Compare July 16, 2023 10:11
@NICO-SOLUTIONS NICO-SOLUTIONS marked this pull request as ready for review July 16, 2023 10:16
@NICO-SOLUTIONS NICO-SOLUTIONS mentioned this pull request Jul 16, 2023
38 tasks
Copy link

@antoniocanovas antoniocanovas left a comment

Choose a reason for hiding this comment

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

Functional review OK !!

Copy link
Contributor

@moitabenfdz moitabenfdz left a comment

Choose a reason for hiding this comment

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

LGTM

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@leemannd
Copy link

/ocabot migration project_sequence

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jul 24, 2023
@leemannd
Copy link

Hello @NICO-SOLUTIONS , Thank you for your contribution.
There is still one little step required. Could you squash together administrative commits and commit related to translations?

Copy link

@leemannd leemannd left a comment

Choose a reason for hiding this comment

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

LGTM Waiting for the squashing of adminstrative/translation commits before merge

@NICO-SOLUTIONS
Copy link
Member Author

NICO-SOLUTIONS commented Jul 24, 2023

translations into the IMP pre-commit stuff commit?
Unbenannt

@leemannd
Copy link

Into this wiki, you'll find all main point to be done during migration, especially a full (long) explanation on the squashing of commits -> https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 16.0-mig-project_sequence branch from d106bb4 to 34244c0 Compare July 24, 2023 16:09
@NICO-SOLUTIONS
Copy link
Member Author

NICO-SOLUTIONS commented Jul 24, 2023

@leemannd
do you mind to have a look to it, if i have done it correct?

@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 16.0-mig-project_sequence branch 2 times, most recently from 9641d8d to 1e5ca90 Compare July 25, 2023 10:42
@leemannd
Copy link

Hello, it seems something went wrong with rebase. There are a lot of commits that aren't related to this migration

@NICO-SOLUTIONS
Copy link
Member Author

Hello, it seems something went wrong with rebase. There are a lot of commits that aren't related to this migration

That's because my translation commits do not refer to the migration of this module. Translations referring to other modules of this repo. That's why I was confused by squashing suggestion.

OCA-git-bot and others added 3 commits July 25, 2023 15:13
When a project has a name, still the sequence is a very important field to be displayed. I move it below the project name.

@moduon MT-1506
`hr_timesheet` creates an analytic account by default. The method it uses to create it expects a preexisting name. But since we're making name not required, we're breaking other module's expectations.

To fix this problem, now the name sync is done before writing or creating records, and not after.

To make sure the problem doesn't happen anymore, we keep the `NOT NULL` requirement on project names. We just do it with a manual SQL constraint. This extra protection ensures compatibility with any other modules that expect always a value on the name.

Any possibly misconfigured sequence could produce sequence duplicates. I also add protection against that.

In tests, the project sequence was wrongly reset to 11 only once. Turns out that it survives the test savepoint, so I now reset it before each test instead. Tests are more reliable now. Besides, I added some more.

@moduon MT-1506

wip

wip
@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 16.0-mig-project_sequence branch from 1e5ca90 to b516b82 Compare July 25, 2023 13:16
@NICO-SOLUTIONS
Copy link
Member Author

Hello, it seems something went wrong with rebase. There are a lot of commits that aren't related to this migration

@leemannd
as is think this probably was a misunderstanding. i switched back to the previous state (with fresh rebase onto 16.0). If i understood correctly. the translation commits have to be squashed in general (as well as administrative commits). As the translations made by me are referred to other modules, i think sqashing them is not the right way here.

If there is anything unclear to me, please open my eyes. Sorry for the hassle... Still improving my git skills. Learned a lot about interactive rebasing now ;P

@pedrobaeza
Copy link
Member

Let me show you graphically the expected squashing:

Selección_141

yajo and others added 6 commits July 26, 2023 11:07
These projects shouldn't display "False - Project name" in their display names.

@moduon MT-1506

[UPD] Update project_sequence.pot

[UPD] README.rst

project_sequence 14.0.0.1.1
To change the project display name pattern, follow these steps:

1. Go to *Project > Configuration > Settings*.
2. Edit the *Project display name pattern* field.

   The default format is ``%(sequence_code)s - %(name)s``. You can use those
   same placeholders to customize the pattern.

@moduon MT-1506

[UPD] Update project_sequence.pot

[UPD] README.rst

project_sequence 14.0.0.2.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: project-14.0/project-14.0-project_sequence
Translate-URL: https://translation.odoo-community.org/projects/project-14-0/project-14-0-project_sequence/
@moduon MT-2822

[UPD] Update project_sequence.pot

[UPD] README.rst

project_sequence 15.0.0.2.1
Translated using Weblate (Italian)

Currently translated at 100.0% (9 of 9 strings)

Translation: project-15.0/project-15.0-project_sequence
Translate-URL: https://translation.odoo-community.org/projects/project-15-0/project-15-0-project_sequence/it/
@NICO-SOLUTIONS NICO-SOLUTIONS force-pushed the 16.0-mig-project_sequence branch from b516b82 to a941264 Compare July 26, 2023 09:08
@NICO-SOLUTIONS
Copy link
Member Author

@pedrobaeza
Thanks you pedro for the graphical explanation. I was focussed to much on the translations. Got it now!

@leemannd
sorry for the hassle

@leemannd
Copy link

@NICO-SOLUTIONS Thank you for the work

@leemannd
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1155-by-leemannd-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit fe78869 into OCA:16.0 Jul 26, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 00ae9c6. Thanks a lot for contributing to OCA. ❤️

@NICO-SOLUTIONS NICO-SOLUTIONS deleted the 16.0-mig-project_sequence branch July 26, 2023 09:36
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.

8 participants