Skip to content

[18.0][ADD] website_mass_mailing_altcha - #1218

Open
jans23 wants to merge 1 commit into
OCA:18.0from
Nitrokey:18.0-add-website_mass_mailing_altcha
Open

[18.0][ADD] website_mass_mailing_altcha#1218
jans23 wants to merge 1 commit into
OCA:18.0from
Nitrokey:18.0-add-website_mass_mailing_altcha

Conversation

@jans23

@jans23 jans23 commented Sep 12, 2026 •
edited
Loading

Copy link
Copy Markdown

This module adds ALTCHA support to the newsletter subscription snippets.

Depends on

@OCA-git-bot OCA-git-bot added series:18.0 mod:website_mass_mailing_altcha Module website_mass_mailing_altcha labels Sep 12, 2026
@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch 2 times, most recently from 8e0d3d9 to 5e0443e Compare September 12, 2026 17:55
@jans23 jans23 changed the title [ADD] website_mass_mailing_altcha [18.0][ADD] website_mass_mailing_altcha Sep 12, 2026
@jans23
jans23 marked this pull request as ready for review September 12, 2026 18:48
@jans23

jans23 commented Sep 12, 2026

Copy link
Copy Markdown
Author

@hbrunn maybe you would be willing to review this?

hbrunn
hbrunn requested changes Sep 18, 2026
View reviewed changes
Comment thread website_mass_mailing_altcha/static/src/newsletter.esm.js Outdated
Comment thread website_mass_mailing_altcha/static/src/newsletter.esm.js Outdated
}
},

async altcha_get_payload() {

@hbrunn hbrunn Sep 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

all of the following is bullshit. the right thing to do here is override just _onSubscribeClick, wait for verification, attach to the rpc bus to inject the result (and detach the function afterwards), then call super. ~10loc, not 100

@jans23 jans23 Sep 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed as requested.

@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch from 5e0443e to 2d6c991 Compare September 18, 2026 06:38
@OCA-git-bot OCA-git-bot added the mod:website_altcha Module website_altcha label Sep 18, 2026
@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch 2 times, most recently from 4179487 to 262da5e Compare September 18, 2026 07:03
@OCA-git-bot OCA-git-bot removed the mod:website_altcha Module website_altcha label Sep 18, 2026
@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch from 262da5e to 5afb413 Compare September 18, 2026 07:08
hbrunn
hbrunn requested changes Sep 18, 2026
View reviewed changes
Comment thread website_mass_mailing_altcha/static/src/newsletter.esm.js Outdated
.append(renderToString("website_altcha.AltchaWidget", {}));
}
},
});

@hbrunn hbrunn Sep 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not what I asked for

@jans23 jans23 Sep 24, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@hbrunn Did I get it now?

@hbrunn hbrunn Sep 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

code is fine now, the change to website_altcha should go in its own commit though.

and you need to follow https://github.com/OCA/.github/blob/master/AI_POLICY.md

@jans23 jans23 Sep 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

@hbrunn hbrunn Sep 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can have multiple commits in one PR. anyways. because it's always just as grating as here, I don't usually review the output of bullshit generators. don't ask me to do that again.

@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch from 5afb413 to 7b96db6 Compare September 24, 2026 11:50
@OCA-git-bot OCA-git-bot added the mod:website_altcha Module website_altcha label Sep 24, 2026
@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch from 7b96db6 to 7dadf32 Compare September 25, 2026 08:01
@OCA-git-bot OCA-git-bot removed the mod:website_altcha Module website_altcha label Sep 25, 2026
hbrunn
hbrunn approved these changes Sep 25, 2026
View reviewed changes

@hbrunn hbrunn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fine after merge of dependency PR

tarteo
tarteo approved these changes Oct 16, 2026 •
edited
Loading
View reviewed changes
tarteo
tarteo requested changes Oct 16, 2026
View reviewed changes

@tarteo tarteo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When saving the page with this snippet, it rerenders the altcha element everytime. I think you could use cleanForSave for this.
Image

@jans23
[ADD] website_mass_mailing_altcha
0bc2b9c 
Assisted-by: Claude Opus 5.5 (1M context)
@jans23
jans23 force-pushed the 18.0-add-website_mass_mailing_altcha branch from 7dadf32 to 0bc2b9c Compare October 16, 2026 15:26
@jans23

jans23 commented Oct 16, 2026

Copy link
Copy Markdown
Author

@tarteo Thank you for the feedback. Now I added destroy() instead of cleanForSave because it works also in cases where the newsletter block is not clicked during editing.

@tarteo

tarteo commented Oct 17, 2026

Copy link
Copy Markdown
Member

@jans23 Hey, I'm not in the office until Thursday, so atm I don't have time to retest it. Please move this PR forward if this is confirmed fixed by somebody else.

@ByteMeAsap

ByteMeAsap commented Oct 18, 2026

Copy link
Copy Markdown

@tarteo Thank you for the feedback. Now I added destroy() instead of cleanForSave because it works also in cases where the newsletter block is not clicked during editing.

Its not even showing the altcha widget even in non editable mode.

Instead we can add this in publicWidget.registry.subscribe

start: function () { const res = this._super(...arguments); if (this.editableMode) { // We should delete the altcha widget in edit mode this.$el.find(".o_altcha_widget").parents(".s_website_form_custom").remove(); } return res; },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tarteo tarteo tarteo requested changes
@hbrunn hbrunn hbrunn approved these changes

Assignees

No one assigned

Labels

mod:website_mass_mailing_altcha Module website_mass_mailing_altcha series:18.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jans23 @tarteo @ByteMeAsap @hbrunn @OCA-git-bot