[14.0][MIG] l10n_be_cooperator_national_number #69

carmenbianca · 2023-05-23T14:01:08Z

internal task: https://gestion.coopiteasy.be/web#view_type=form&model=project.task&id=10633&active_id=10633&menu_id=

other internal task: https://gestion.coopiteasy.be/web#id=10844&action=475&active_id=107&model=project.task&view_type=form&menu_id=536

depends on OCA/l10n-belgium#190

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

…any_default_get() Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

l10n_be_cooperator_national_number/models/subscription_request.py

l10n_be_cooperator_national_number/views/subscription_request_view.xml

carmenbianca · 2023-05-31T12:26:36Z

@victor-champonnois Needs another review for refactoring

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

In some scenarios, the migration is run on a database that does not yet have the requisite fields in the database. By making it a post-migration script, we avoid the problem. Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

huguesdk

yay, more tests! 👍

huguesdk · 2023-06-23T08:59:21Z

l10n_be_cooperator_national_number/models/company.py

+                        "If the 'Require National Number' toggle is enabled,"
+                        " then so must the 'Display National Number' toggle."


Suggested change

"If the 'Require National Number' toggle is enabled,"

" then so must the 'Display National Number' toggle."

'If the "Require National Number" toggle is enabled,'

' then so must the "Display National Number" toggle.'

i think that delimiting strings with double quotes is more common in user-facing text.

…eld to res.partner

victor-champonnois · 2023-07-14T07:48:26Z

l10n_be_cooperator_national_number/readme/USAGE.rst

+On the company, two new toggles 'Display National Number' and 'Require National
+Number' are added. In order to expose the functionality of this module, you must
+enable these toggles for every company that wishes to use it. 'Display' shows
+the field and allows it to be filled in. 'Require' makes the field mandatory.


Shouldn't we be able to display the field and use it without making it mandatory ? If not what's the point in having two toggles ?

That's what the 'Display' toggle is for. When the 'Display' toggle is disabled, it's as if this module isn't installed. Useful for multi-company.

Ok, it's just that the sentence "In order to expose the functionality of this module, you must enable these toggles for every company that wishes to use it.", seems to mean that you have to enable both toggles for the functionality to work, while actually you can only toggle "display" and the functionality will work (without requirements on the NRN).

Ah, maybe let's write 'one or both of these toggles'?

victor-champonnois · 2023-07-14T07:48:30Z

l10n_be_cooperator_national_number/models/subscription_request.py

        return partner

    def create_coop_partner(self):
        partner = super().create_coop_partner()
-        self.create_national_number(partner)
+        if self.require_national_number:


why putting a condition on "require" and not "display" here ? If the require config is unchecked but display is checked it will not write the national number on the partner ?

Good call. I will patch this up, because simply switching to display won't suffice.

This was surprisingly difficult to fix. Done now. Ready for another review.

Code and functional review OK

…when not required

OCA-git-bot · 2023-07-18T15:00:03Z

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). 🤖

github-actions · 2024-01-07T12:27:10Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

dreispt · 2024-05-02T15:17:50Z

/ocabot merge nobump

OCA-git-bot · 2024-05-02T15:17:58Z

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-69-by-dreispt-bump-nobump, awaiting test results.

OCA-git-bot · 2024-05-02T15:49:09Z

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

victor-champonnois and others added 9 commits May 23, 2023 15:45

[ADD] l10n_be_cooperator_national_number

dae21b8

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

[ADD] l10n_be_cooperator_website_national_number

6716e31

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

run black

dbe9d7b

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

[UPD] Update l10n_be_cooperator_national_number.pot

774df4c

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

[UPD] README.rst

26ece8b

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

[ADD] icon.png

d0653c3

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

[IMP] l10n_be_cooperator_national_number: black, isort, prettier

6f11e56

[MIG] l10n_be_cooperator_national_number: Migration to 13.0

78089d8

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

[MIG] l10n_be_cooperator_national_number: Migration to 14.0

b32b305

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

carmenbianca mentioned this pull request May 23, 2023

[14.0][MIG] l10n_be_national_number OCA/l10n-belgium#190

Merged

[FIX] l10n_be_cooperator_national_number: Stop using deprecated _comp…

4d63763

…any_default_get() Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

carmenbianca mentioned this pull request May 24, 2023

[14.0][MIG] l10n_be_cooperator_website_national_number #70

Draft

carmenbianca commented May 24, 2023

View reviewed changes

victor-champonnois approved these changes May 25, 2023

View reviewed changes

carmenbianca mentioned this pull request May 26, 2023

[14.0] l10n_be_cooperator_national_number: Weird association between 'require' and 'display' #71

Open

carmenbianca marked this pull request as ready for review May 31, 2023 08:55

carmenbianca force-pushed the 14.0-mig-l10n_be_cooperator_national_number branch 4 times, most recently from 6718890 to b124beb Compare May 31, 2023 12:24

carmenbianca requested a review from victor-champonnois May 31, 2023 12:26

[REF] l10n_be_cooperator_national_number: Separate display from require

0393cae

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>

carmenbianca force-pushed the 14.0-mig-l10n_be_cooperator_national_number branch from b124beb to 0393cae Compare June 15, 2023 14:05

huguesdk reviewed Jun 23, 2023

View reviewed changes

[IMP] use double quotes in user-facing message

cfd17fd

huguesdk approved these changes Jun 23, 2023

View reviewed changes

OCA-git-bot added the approved label Jun 23, 2023

[FIX] l10n_be_cooperator_national_number: Don't write non-existent fi…

d585d9a

…eld to res.partner

victor-champonnois suggested changes Jul 14, 2023

View reviewed changes

OCA-git-bot removed the approved label Jul 14, 2023

[FIX] l10n_be_cooperator_national_number: Write national number even …

10afd2e

…when not required

carmenbianca requested a review from victor-champonnois July 17, 2023 10:20

carmenbianca force-pushed the 14.0-mig-l10n_be_cooperator_national_number branch from f45e99e to 2fb232c Compare July 17, 2023 12:40

[FIX] l10n_be_cooperator_national_number: Fix the tests

69057a4

carmenbianca force-pushed the 14.0-mig-l10n_be_cooperator_national_number branch from bf7c168 to 69057a4 Compare July 17, 2023 13:52

victor-champonnois approved these changes Jul 18, 2023

View reviewed changes

OCA-git-bot added approved ready to merge labels Jul 18, 2023

github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jan 7, 2024

github-actions bot closed this Feb 11, 2024

huguesdk added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Feb 12, 2024

huguesdk reopened this Feb 12, 2024

OCA-git-bot added the bot is merging ⏳ label May 2, 2024

OCA-git-bot merged commit f37dc36 into OCA:14.0 May 2, 2024
8 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14.0][MIG] l10n_be_cooperator_national_number #69

[14.0][MIG] l10n_be_cooperator_national_number #69

carmenbianca commented May 23, 2023 •

edited

Loading

carmenbianca commented May 31, 2023

huguesdk left a comment

huguesdk Jun 23, 2023

victor-champonnois Jul 14, 2023

carmenbianca Jul 17, 2023

victor-champonnois Jul 18, 2023 •

edited

Loading

carmenbianca Jul 18, 2023

victor-champonnois Jul 18, 2023

victor-champonnois Jul 14, 2023

carmenbianca Jul 17, 2023

carmenbianca Jul 17, 2023

victor-champonnois Jul 18, 2023

OCA-git-bot commented Jul 18, 2023

github-actions bot commented Jan 7, 2024

dreispt commented May 2, 2024

OCA-git-bot commented May 2, 2024

OCA-git-bot commented May 2, 2024

		"If the 'Require National Number' toggle is enabled,"
		" then so must the 'Display National Number' toggle."

[14.0][MIG] l10n_be_cooperator_national_number #69

[14.0][MIG] l10n_be_cooperator_national_number #69

Conversation

carmenbianca commented May 23, 2023 • edited Loading

carmenbianca commented May 31, 2023

huguesdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

victor-champonnois Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OCA-git-bot commented Jul 18, 2023

github-actions bot commented Jan 7, 2024

dreispt commented May 2, 2024

OCA-git-bot commented May 2, 2024

OCA-git-bot commented May 2, 2024

carmenbianca commented May 23, 2023 •

edited

Loading

victor-champonnois Jul 18, 2023 •

edited

Loading