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

Cookies & GDPR #417

Merged
merged 2 commits into from
Dec 27, 2021
Merged

Cookies & GDPR #417

merged 2 commits into from
Dec 27, 2021

Conversation

patriksh
Copy link
Contributor

@patriksh patriksh commented Oct 7, 2021

Related issues: #211 #245

How do I add new email templates so they work properly with updating Osclass?

How to disable sanitization on form labels so I can add tags to terms & conditions / privacy policy?

Do I add additional columns to both user and item table regarding #269? We already have b_show_email for item, do I add b_show_phone? What about showing the same data on public profile?

@alastairR
Copy link

I don't know if it's any use, but I merged a GDPR cookie popup plugin with an old GDPR osclass plugin to create a new plugin that I just published yesterday: https://github.com/alastairR/osclass-gdpr

I suspect that other than a core option in listing edit to hide a phone number, like the email hide, the rest of the GDPR stuff should be in an open source plugin.

@navjottomer
Copy link
Member

navjottomer commented Oct 8, 2021

@alastairR

I don't know if it's any use, but I merged a GDPR cookie popup plugin with an old GDPR osclass plugin to create a new plugin that I just published yesterday: https://github.com/alastairR/osclass-gdpr

I suspect that other than a core option in listing edit to hide a phone number, like the email hide, the rest of the GDPR stuff should be in an open source plugin.

That's great.
I was sceptical about merging this PR in our beta as I don't have time to test all the changes. I was thinking of merging it after 5.1 but a plugin is much better.

@dftd

How do I add new email templates so they work properly with updating Osclass?

How to disable sanitization on form labels so I can add tags to terms & conditions / privacy policy?

The current option is to use DAO methods for inserting email templates.
If you're using FormInputs class, it doesn't use any sanitization on input labels.

@patriksh
Copy link
Contributor Author

patriksh commented Oct 8, 2021

@navjottomer I will add the email template + some other fixes.

I thibk this should surely be a part of Osclass core, doesn't matter if it won't make it to 5.1 release.

I use FormInputs class, check this:

return '<label class="' . $class . '" for="' . $for . '">' . $this->escape::html($label) . '</label>';

Label escapes html no matter what options are set on the input.

Also regarding checkboxes, what classes do I put on the inputs, do I make it Bender compatible out-of-the-box?
Ideally themes would implenent checkboxes themselves and remove the default hooks.

@navjottomer
Copy link
Member

I thibk this should surely be a part of Osclass core, doesn't matter if it won't make it to 5.1 release.

We'll see about it, for meantime, a plugin is already available so that's good for us.

I use FormInputs class, check this:

return '<label class="' . $class . '" for="' . $for . '">' . $this->escape::html($label) . '</label>';

Yup, it's escaping but you said sanitization got me confused. Anyway, it will only escape not remove any tag.

Also regarding checkboxes, what classes do I put on the inputs, do I make it Bender compatible out-of-the-box? Ideally themes would implenent checkboxes themselves and remove the default hooks.

Keep the default and it'll work with bender. If theme developers want to change classes then they can just extend the Form classes, override the properties and call the same method with extended class, that's super easy for them.

@navjottomer navjottomer merged commit c0f9c9d into mindstellar:develop Dec 27, 2021
@patriksh
Copy link
Contributor Author

Oh, how come you merged it... I gotta add some fixes.
Do I make a new PR?

@navjottomer
Copy link
Member

navjottomer commented Dec 27, 2021 via email

@patriksh
Copy link
Contributor Author

patriksh commented Dec 27, 2021

@navjottomer Again having problems with escaping HTML.

If I use sprintf(__('I agree with the <a href="%s">Privacy Policy</a>.'), osc_gdpr_privacy_page_url()) as label, it does get removed since label function is using $this->escape::html($label).

Even $options['customHtml'] gets printed out as plain text instead of HTML.

@patriksh
Copy link
Contributor Author

No idea what's the best way to insert the new mail template. Do I hardcode a function for it and run it when upgrade DB is ran?
Do you think cookie notice should be translatable inside admin? And I save translations as JSON in preference? Or we just keep it as it is, so it can be translated in .po files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants