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

Add privacy statement to relevant places #941

Merged
merged 3 commits into from
May 31, 2024
Merged

Add privacy statement to relevant places #941

merged 3 commits into from
May 31, 2024

Conversation

stwalkerster
Copy link
Member

This adds links to the WMCS TOS and our brand new {entry}.php/privacy pages, which render the privacy statement in the correct context from a markdown file. Links have been added to both public and private footers, and to relevant data submission forms (request and register) for when users start using the service.

I've also taken the opportunity to sassify the public stylesheet.

@stwalkerster stwalkerster marked this pull request as ready for review May 27, 2024 01:48
Copy link
Member

@FastLizard4 FastLizard4 left a comment

Choose a reason for hiding this comment

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

Noticed a few different issues, including some small but highly consequential casing errors, plus a question or two; comments in-line.

Also noticed some minor UI issues, though I'd be okay with addressing these in a follow-up PR given the time constraints we're under.

Margin under the footer on public pages is missing (that green line is the top of my taskbar; I can't scroll down any further):
image

Similarly missing on internal pages:
image

Just as an FYI, it looks like our markdown renderer can't do tables. Not that I was planning to use any, but maybe something we want to fix one day?
image

Also as an FYI, I've been getting some new deprecation warnings when running ACC locally. Seems to be related to recent Smarty updates:

image

includes/Router/PublicRequestRouter.php Outdated Show resolved Hide resolved
includes/SiteConfiguration.php Show resolved Hide resolved
includes/SiteConfiguration.php Outdated Show resolved Hide resolved
@@ -0,0 +1,7 @@
<div class="row">
<p class="legal-info">
By submitting this form, you agree to your information being processed to fulfil your request in
Copy link
Member

Choose a reason for hiding this comment

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

TIL that you spell "fulfill" incorrectly in addition to "color". ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Genuine question - does this legally matter? I can't recall if we have a defined preferred ENGVAR in the tool at all.

I can't imagine it being important immediately, but it might be something to come back to.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm really not sure. Technically the legal venue here would be the US, but I have a hard time believing that the use of British English would nullify the agreement.

I would say let's kick this down the road to a larger discussion of what ENGVAR we want to use; I can't see this being any issue in the near-term future. (Or if it is, I'm sure we'll find out about it.)

templates/base.tpl Outdated Show resolved Hide resolved
templates/publicbase.tpl Outdated Show resolved Hide resolved
templates/registration/register.tpl Outdated Show resolved Hide resolved
templates/request/legal-info.tpl Outdated Show resolved Hide resolved
templates/request/request-form.tpl Outdated Show resolved Hide resolved
@@ -34,6 +34,7 @@
{$formCommentsHelp}
</small>
</div>
{include file="request/legal-info.tpl"}
Copy link
Member

Choose a reason for hiding this comment

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

Should this be customizable as with the rest of the form? Thinking about use cases like the ruwiki form, though maybe you've already thought about that and this is a v2 thing?

Copy link
Member Author

@stwalkerster stwalkerster May 30, 2024

Choose a reason for hiding this comment

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

I hadn't thought about that, but the policy says "in English (as well as any other language versions if applicable)". For now, I think a gtranslate of the legal-info.tpl can be added to the existing dynamic header just fine, leaving the hard-coded English version here.

If we want to make this truly dynamic in a nice way, it's going to be a schema change (just nope).

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. I think it would be a good idea to make it dynamic Eventually, but there's other multidomain stuff that I think is more impactful.

@stwalkerster
Copy link
Member Author

Margin under the footer on public pages is missing

Not critical for now, but acknowledged for later.

Just as an FYI, it looks like our markdown renderer can't do tables.

Yes it can, we just don't have it enabled.

Also as an FYI, I've been getting some new deprecation warnings when running ACC locally. Seems to be related to recent Smarty updates:

Already tracked as #794

This adds links to the WMCS TOS and our brand new {entry}.php/privacy
pages, which render the privacy statement in the correct context from a
markdown file. Links have been added to both public and private footers,
and to relevant data submission forms (request and register) for when
users start using the service.

I've also taken the opportunity to sassify the public stylesheet.
@stwalkerster
Copy link
Member Author

I think that's mostly all fixed now. I've even enabled Markdown tables for you.

Copy link
Member

@FastLizard4 FastLizard4 left a comment

Choose a reason for hiding this comment

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

:shipit:

@stwalkerster stwalkerster merged commit c4e965c into master May 31, 2024
4 checks passed
@stwalkerster stwalkerster deleted the tos-privacy branch May 31, 2024 07:03
@stwalkerster stwalkerster linked an issue May 31, 2024 that may be closed by this pull request
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

WMCS Terms of Use update
2 participants