Skip to content

[18.0][IMP] subscription_oca : add a setting to choose if the subscription … - #1404

Open
cvinh wants to merge 1 commit into
OCA:18.0from
invitu:18.0-imp_subscription_oca_no_auto_start
Open

cvinh wants to merge 1 commit into
OCA:18.0from
invitu:18.0-imp_subscription_oca_no_auto_start

Conversation

@cvinh

@cvinh cvinh commented Mar 12, 2026

Copy link
Copy Markdown

…will start automatically when sale order is confirmed or not

@luisDIXMIT luisDIXMIT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review and LGTM!

@rrebollo rrebollo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

From a technical perspective, LGTM.

Would you be open to adding a test covering the change you introduced to the "standard" feature? Before this PR, my understanding from the code is that every newly created subscription automatically started. Now, with your setting, that won't happen by default.

I think adding a test would be a good safety measure—let's see how it goes. You can ping me then.

Also, would you be so kind to review my #1410 in return?

@cvinh
cvinh force-pushed the 18.0-imp_subscription_oca_no_auto_start branch from 3849115 to 3c883ff Compare April 21, 2026 20:21
@cvinh

cvinh commented Apr 21, 2026

Copy link
Copy Markdown
Author

From a technical perspective, LGTM.

Would you be open to adding a test covering the change you introduced to the "standard" feature? Before this PR, my understanding from the code is that every newly created subscription automatically started. Now, with your setting, that won't happen by default.

I think adding a test would be a good safety measure—let's see how it goes. You can ping me then.

Also, would you be so kind to review my #1410 in return?

tests added

@cvinh

cvinh commented Apr 21, 2026

Copy link
Copy Markdown
Author

I don't understand why tests fail

@rrebollo rrebollo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review. LGTM!

Comment thread subscription_oca/__manifest__.py Outdated
@rrebollo

Copy link
Copy Markdown

@cvinh did you rebase? The tests might pass then.

@github-actions

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Aug 30, 2026
@cvinh
cvinh force-pushed the 18.0-imp_subscription_oca_no_auto_start branch from 3c883ff to c9baf0c Compare August 30, 2026 16:48
…will start automatically when sale order is conformed or not
@cvinh
cvinh force-pushed the 18.0-imp_subscription_oca_no_auto_start branch from c9baf0c to 3e5a218 Compare August 30, 2026 16:52
@github-actions github-actions Bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 6, 2026
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.

4 participants