Feat/add optional policies #51

AngryDragonite · 2024-04-27T23:06:06Z

Resolves #33

Changes proposed in this pull request:

First of all, I added migrations that create columns: optional (boolean) and additional_info (JSON) inside fof_terms_policies table. Table fof-terms-policy-user has been enriched with is_accepted (boolean) column.

Modifications inside extend.php:

added styles to forum
added controller that handles rejection of an (optional) policy,
added relationship fofTermsPoliciesState that allows to check which policies were accepted,
removed Registered event listener, its functionality is now being handled inside RegisterMiddleware.php.

Modifications inside Policy.php, UserPolicyData.php, PolicyStoreController.php, PolicyRepository.php, RegisterPolicyValidator.php:

generally speaking, added functionality that would integrate backend with new columns introduced in migrations,
validator now forces users to accept only non-optional policies.

Modifications inside RegisterMiddleware.php:

added functionality that handles storing the state of user's policies (wheter a policy was accepted, or not) to database.

New backend files:

PolicyDeclineController.php - as the name suggests. Basically, it is "1:1 copy" of PolicyAcceptController.php, however with opposite behaviour

Now lets take a look at what changed in the frontend.

Modifications inside Policy.js:

added optional and additional_info fields to the Policy model.

Modifications inside PolicyEdit.js:

added new item to fields method, which allows to choose, wheter specified Policy should be optional, or not,
synced initNewField method with Policy model changes,
modified deletePolicy method - during developing I encountered a bug - when deleting a Policy, its field is, of course deleted, but also savePolicy method is being called. It was happening possibly because when clicking Delete policy button, onsubmit method is being called.

Modifications inside AcceptPoliciesModal.js:

added functionality that handles rejecting an optional policy.

Modifications inside `forum/index.js':

instantiating new addManagePoliciesOption component.

Modifications inside admin/components/index.js:

exporting new ExtensionData component, so developers working on integrating fof/terms with other extensions could access this component

New frontend files:

ExtensionData.js - a component that allows developers to integrate fof/terms with other extensions by saving their data into fof-terms-policies table. Recipe on how to use it is descriped in README.md in For developers section.
addManagePoliciesOption.js - a component that allows users to opt in/out of consents in their profile settings.

Other changes:

admin.less & new forum.less - added some styling for components,
en.yml - added translations.
README.md - added section For developers, explaining step-by-step how to use ExtensionData component, and therefore integrate fof/terms with other extensions.

Reviewers should focus on:

I would definitely want to know, if my instructions on how to use ExtensionData in README.md are specific, exact and free of ambiguities. Every feedback on what in my code can be fixed, rewritten so it is easier to read, or is simply better is greatly welcomed :)

Screenshot

admin panel - select policy as optional

user settings - opt in/out of consents

Confirmed

Frontend changes: tested on a local Flarum installation.
Backend changes: tests are green (run composer test).

Required changes:

Related Flarum core extension PR's: (Omit this section if irrelevant)

feat: added 'is_accepted' field to fof-terms-policy-user table feat: added settings to opt out of consents in user profile feat: added functionality that allows users to decline policies that were declared optional

feat: added 'ExtensionData' component

…ically feat: changed 'ExtensionData' component, so it allows other extensions to sync with fof/terms fix: synced new model field name to its DB column

feat: started writing guide on how to integrate fof/terms with other extensions in README.md

feat: added translations feat: improved ExtensionData component feat: added recipe for integrating fof/terms with other extensions in README fix: additionalInfo is now properly saved into DB chore: removed redundant imports and comments

…cies.

… now saved properly.

DavideIadeluca

Thanks @AngryDragonite for your contribution! Just some minor things to iron out :)

migrations/2024_11_4_07_edit_terms_policies.php

extend.php

resources/locale/en.yml

js/src/forum/components/addManagePoliciesOption.js

Co-authored-by: Davide Iadeluca <146922689+DavideIadeluca@users.noreply.github.com>

… feat/add-optional-policies

chore: removed accidently added files

DavideIadeluca

Thanks @AngryDragonite for the changes. The migrations still need a look, otherwise this PR from my side is good for a merge

migrations/2024_11_4_124021_edit_terms_policies.php

migrations/2024_11_4_124110_edit_terms_policy_user.php

migrations/2024_23_04_124250_edit_terms_policies.php

AngryDragonite · 2024-06-07T14:52:24Z

All requested changes committed.

imorland

Thanks for the PR @AngryDragonite and sorry it's taken us so long to look at this.

Overall I think it's okay, but I'd like to give it a spin locally before merging. Please will you remove the committed dist folder from the PR? This is not needed here as these files will be built automatically upon merge, and only cause merge conflicts at this stage.

You can do this easily: git reset HEAD js/dist and then push that change :)

imorland · 2024-06-16T12:31:01Z

@AngryDragonite thanks for your efforts, unfortunately you're now deleting the dist folder contents, rather than simply omitting any changes to them. This should help you ;)

rafaucau · 2024-06-16T14:18:40Z

@imorland There are changes to the dist folder on the master branch since this PR. Perhaps this is causing the issues?
https://github.com/FriendsOfFlarum/terms/commits/master/js/dist?since=2024-04-28&until=2024-06-16

I tried to help @AngryDragonite, but still the files are in this Pull Request.

js/src/forum/components/addManagePoliciesOption.js

src/Middlewares/RegisterMiddleware.php

imorland · 2024-06-16T15:51:33Z

src/Middlewares/RegisterMiddleware.php

            }
        }

-        return $handler->handle($request);
+        return $response;


Any particular reason why we're not continuing down the middleware stack here?

This is now handled earlier above (line 64). We need to handle this to read response body. GitHub diff is messed

terms/src/Middlewares/RegisterMiddleware.php

Line 64 in 78fef16

$response = $handler->handle($request);

Github diff is messed in this file. Please review the entire file without viewing the diff.

migrations/2024_04_24_01_edit_terms_policies.php

js/src/forum/components/addManagePoliciesOption.js

…efault value to false for new users

rafaucau · 2024-07-18T09:27:20Z

ping @imorland @DavideIadeluca

rafaucau · 2024-07-30T15:02:59Z

Maybe someone else from the FOF team could review this PR? Like, for example, @dsevillamartin
It has been waiting since April, while changes are being made to the main branch of this repo, which may cause merging conflicts.

DavideIadeluca

Thank you for your patience. I had another look again and from my side this is good for a merge to master. Everything is working as expected locally.

About the merge conflicts, before we merge this to master I think the following should work to resolve the issue:

git fetch origin
git checkout origin/master -- js/dist/admin.js js/dist/admin.js.map js/dist/forum.js js/dist/forum.js.map
git commit -m "chore: resolve merge conflict"
git push origin feat/add-optional-policies

rafaucau · 2024-07-31T08:35:22Z

@DavideIadeluca merging issue resolved

rafaucau · 2024-08-02T10:11:57Z

@DavideIadeluca So what about merge?

DavideIadeluca · 2024-08-02T10:12:55Z

@DavideIadeluca So what about merge?

Awaiting final approval from @imorland

AngryDragonite · 2024-08-15T09:54:52Z

ping @imorland

AngryDragonite · 2024-08-26T15:50:13Z

@imorland @DavideIadeluca is there any chance of getting this thing done in any near future? We've been waiting for 4 months.

AngryDragonite and others added 12 commits April 19, 2024 16:07

feat: added 'optional' field to fof-terms-policies table

dd03bba

feat: added 'is_accepted' field to fof-terms-policy-user table feat: added settings to opt out of consents in user profile feat: added functionality that allows users to decline policies that were declared optional

feat: accept policies really accepted by user instead of all

aef02d4

feat: added 'additionalInfo' model attribute

a912534

feat: added 'ExtensionData' component

feat: changes in policy consents in profile settings now save automat…

39874c5

…ically feat: changed 'ExtensionData' component, so it allows other extensions to sync with fof/terms fix: synced new model field name to its DB column

refactor: minor changes in 'ExtensionData' and 'PolicyEdit'

668020d

feat: started writing guide on how to integrate fof/terms with other extensions in README.md

refactor: simplified addManagePoliciesOption code

c60cfce

feat: added translations feat: improved ExtensionData component feat: added recipe for integrating fof/terms with other extensions in README fix: additionalInfo is now properly saved into DB chore: removed redundant imports and comments

Apply fixes from StyleCI

e921f1b

refactor: changed additionalInfo column name into additional_info

9f58cdd

fix: fixed bug that caused inappropriate behaviour when deleting poli…

966e00b

…cies.

style: formatted README.md

9285ab7

fix: optional policies that were not accepted during registration are…

69cb8db

… now saved properly.

chore: added phpstan ignore to User::$fofTermsPoliciesState.

df7e49a

DavideIadeluca requested changes May 25, 2024

View reviewed changes

DavideIadeluca assigned AngryDragonite May 25, 2024

DavideIadeluca requested a review from dsevillamartin May 25, 2024 12:15

AngryDragonite and others added 7 commits May 28, 2024 12:19

Update resources/locale/en.yml

522a965

Co-authored-by: Davide Iadeluca <146922689+DavideIadeluca@users.noreply.github.com>

Update resources/locale/en.yml

35ebdac

Co-authored-by: Davide Iadeluca <146922689+DavideIadeluca@users.noreply.github.com>

chore: minor changes in addManagePoliciesOption.js

379d294

chore: renamed migration files

f4c1a84

Merge remote-tracking branch 'origin/feat/add-optional-policies' into…

fb1e59e

… feat/add-optional-policies

chore: revert file permissions

5ea2c61

chore: reverted file permissions

c0718a3

chore: removed accidently added files

AngryDragonite requested a review from DavideIadeluca May 28, 2024 11:04

DavideIadeluca requested changes Jun 6, 2024

View reviewed changes

migrations/2024_11_4_124021_edit_terms_policies.php Outdated Show resolved Hide resolved

migrations/2024_11_4_124110_edit_terms_policy_user.php Outdated Show resolved Hide resolved

migrations/2024_23_04_124250_edit_terms_policies.php Outdated Show resolved Hide resolved

chore: fixed migration files' names

22ea58a

DavideIadeluca approved these changes Jun 7, 2024

View reviewed changes

DavideIadeluca requested review from imorland and removed request for dsevillamartin June 15, 2024 18:22

imorland requested changes Jun 16, 2024

View reviewed changes

chore: sync fork and reset dist and package.json files

84721f8

DavideIadeluca reviewed Jun 16, 2024

View reviewed changes

js/src/forum/components/addManagePoliciesOption.js Show resolved Hide resolved

DavideIadeluca self-requested a review June 16, 2024 14:22

style: reformat addManagePoliciesOption.js

78fef16

DavideIadeluca approved these changes Jun 16, 2024

View reviewed changes

DavideIadeluca requested a review from imorland June 16, 2024 14:42

imorland requested changes Jun 16, 2024

View reviewed changes

rafaucau and others added 9 commits June 16, 2024 18:07

remove var_dump

8e383e3

style: one array access method

2e5efe5

Apply fixes from StyleCI

87fcbbb

style: use link component

a77322a

fix: replace JSON column with longText for compatibility

5634c9c

refactor: simplify migrations

9c631a6

Apply fixes from StyleCI

02bf04b

fix: mark existing policies as accepted for all users, then set the d…

2d2570d

…efault value to false for new users

Apply fixes from StyleCI

b950336

AngryDragonite requested a review from imorland June 17, 2024 09:06

AngryDragonite requested a review from DavideIadeluca July 27, 2024 10:39

DavideIadeluca approved these changes Jul 31, 2024

View reviewed changes

chore: resolve merge conflict

c276a71

imorland merged commit aa2f624 into FriendsOfFlarum:master Sep 29, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/add optional policies #51

Feat/add optional policies #51

AngryDragonite commented Apr 27, 2024 •

edited

Loading

DavideIadeluca left a comment

DavideIadeluca left a comment

AngryDragonite commented Jun 7, 2024

imorland left a comment

imorland commented Jun 16, 2024

rafaucau commented Jun 16, 2024

imorland Jun 16, 2024

rafaucau Jun 16, 2024 •

edited

Loading

rafaucau commented Jul 18, 2024

rafaucau commented Jul 30, 2024 •

edited

Loading

DavideIadeluca left a comment •

edited

Loading

rafaucau commented Jul 31, 2024

rafaucau commented Aug 2, 2024

DavideIadeluca commented Aug 2, 2024

AngryDragonite commented Aug 15, 2024

AngryDragonite commented Aug 26, 2024

Feat/add optional policies #51

Feat/add optional policies #51

Conversation

AngryDragonite commented Apr 27, 2024 • edited Loading

DavideIadeluca left a comment

Choose a reason for hiding this comment

DavideIadeluca left a comment

Choose a reason for hiding this comment

AngryDragonite commented Jun 7, 2024

imorland left a comment

Choose a reason for hiding this comment

imorland commented Jun 16, 2024

rafaucau commented Jun 16, 2024

imorland Jun 16, 2024

Choose a reason for hiding this comment

rafaucau Jun 16, 2024 • edited Loading

Choose a reason for hiding this comment

rafaucau commented Jul 18, 2024

rafaucau commented Jul 30, 2024 • edited Loading

DavideIadeluca left a comment • edited Loading

Choose a reason for hiding this comment

rafaucau commented Jul 31, 2024

rafaucau commented Aug 2, 2024

DavideIadeluca commented Aug 2, 2024

AngryDragonite commented Aug 15, 2024

AngryDragonite commented Aug 26, 2024

AngryDragonite commented Apr 27, 2024 •

edited

Loading

rafaucau Jun 16, 2024 •

edited

Loading

rafaucau commented Jul 30, 2024 •

edited

Loading

DavideIadeluca left a comment •

edited

Loading