Resolve "User registration" #2058
No reviewers
Labels
No labels
Security
TeX
auto-update
board
done
board
ready
board
todo
check
delete-eslint-rc-js
check
update-builddeps-package-json
check
update-eslint-rc-js
check
update-gitignore
check
update-merge-request-template
check
update-prettier-ignore
check
update-pyproject-toml
check
update-renovate-json
check
update-tox-ini
part
backend
part
ci
part
docs
part
frontend
part
i18n
part
non-technical
part
packaging
prio
1
prio
2
prio
3
release-mr-5.x
size
large
size
medium
size
small
source
customer
source
customer::fsmw
source
customer::fss
source
customer::teckids
source
downstream
type
breaking
type
bug
type
feature
type
refactoring
workflow
blocked
workflow
confirmed
workflow
current-todo
workflow
discussing
workflow
new-app
workflow
wontfix
No milestone
No project
No assignees
6 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
aleksis/AlekSIS-Core!2058
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "331-user-registration"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #331
Closes #236
Closes #354
Closes #353
Tests
assigned to @nik
requested review from @hansegucker
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
A short notice: Please remember that you will have to take screenshots from all newly added views/templates ;-)
added 2 commits
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
changed the description
added 3 commits
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
Confirm email
Signup
Social connections
Account inactive
Change password
requested review from @ZugBahnHof and removed review request for @hansegucker
Please use a Materialize collection with a line for each account
Please move this button to the right side (https://materializecss.com/collections.html#secondary)
Why is this a warning? You might want to let users know that this is an important action, but the text "Please confirm..." is not a warning.
Please use the msgbox Snippet (https://edugit.org/AlekSIS/official/AlekSIS-Core/-/blob/master/aleksis/core/templates/components/msgbox.html)
Is this the right text?
What is the purpose of this page?
Good icon choice
Why do you use this icon? It doesn't seem like anything related to submitting
I don't think this should be a warning
I don't think this is a warning
Why do you use this icon? It doesn't seem like anything related to submitting
Please use a consistent spelling of email/e-mail/mail
Isn't there a form missing here?
Why do you use this icon? It doesn't seem like anything related to submitting
I don't think a red card is suited for this. Together with the error icon, the user may think there is a problem
Will this button delete all accounts? Because if not, then it should be in the loop and if it is, then that's bad UX.
Why is this ul not inside the file?
Did you try a materialize collection? I believe it may be better, but maybe it's too heavy for this page
Are these classes necessary?
Typo
Fixed, see !2075
added 72 commits
masterCompare with previous version
added 1 commit
Compare with previous version
resolved all threads
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
marked this merge request as ready
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
changed this line in version 22 of the diff
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 9 commits
masterCompare with previous version
changed the description
changed the description
mentioned in issue #331
requested review from @hansegucker and removed review request for @ZugBahnHof
assigned to @hansegucker and unassigned @nik
requested review from @ZugBahnHof and removed review request for @hansegucker
requested review from @hansegucker and removed review request for @ZugBahnHof
There must be a setting to enable/disable each features (in addition to social accoutns and registration, which are already configurable, changing passwords and getting reset links must also be configurable.
Also, most, if not all, of that, should be site preferences rather than settings (if needed by allauth itself, watch out for the lazy_preference stuff used elsewhere to pull preferences into settings).
We do not have consents yet.
We have Python 3! Happy 2021 ❤️!
Why do you create the fields in
__init__instead of just defining them as slots o nthe class?What is this supposed to do? Looking further down, it is used wrongly (
get_user_modelreturns the model class, not a User instance)This should also be configurable. A system that does not allow using social accounts hsould not expose this menu to users.
This should only be loaded if social accoutns are enabled.
Move inside the
ifblock below to only set it if social accounts are enabled.The setting name might get ambibious. As I understand it, this only supports social accounts supported by allauth. Does that include, like, general things like LDAP, etc.?
Should default to the app title as configured in preferences.
Should default to False.
Should contain the configured app name.
Should this be prepended with a text of some sort? Like, what is this listed for?
Should contain the same wording as in the change password template (for example "forgot" vs. "forgotten")
troblue?
Also, remove the comma before the if.
"No newline at end of file"
I somehow remember writing this. Seems to have been lost in a squash or something? Please restore.
Why don't we use a generic validator that can validate any given setting? Seems unlikely that we want to add one funtion for each and every setting.
Can you also please provide a new screenshot for the current "Social accounts" page?,
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
changed this line in version 25 of the diff
added 1 commit
Compare with previous version
added 11 commits
masterCompare with previous version
I think there must be one somewhere.
Please take a look at the SignupForm from django-allauth
https://github.com/pennersr/django-allauth/blob/master/allauth/account/forms.py#L394
changed this line in version 27 of the diff
added 1 commit
Compare with previous version
changed this line in version 28 of the diff
changed this line in version 28 of the diff
changed this line in version 28 of the diff
changed this line in version 28 of the diff
changed this line in version 28 of the diff
changed this line in version 28 of the diff
changed this line in version 28 of the diff
added 5 commits
Compare with previous version
added 89 commits
masterCompare with previous version
added 1 commit
Compare with previous version
resolved all threads
added 2 commits
96edb165- Install and configure django-allauth.d0a52502- Use custom socialaccount adapter to accommodate LDAP backendCompare with previous version
aborted the automatic merge because source branch was updated
added 1 commit
Compare with previous version
added 1 commit
b069f624- Add missing preferenceCompare with previous version
added 1 commit
eb4cf5e1- Add missing settingsCompare with previous version
added 1 commit
6e2a7e33- Remove logout confirmationCompare with previous version
This would be nice
resolved all threads
added 1 commit
929c0d71- Add missing templatesCompare with previous version
changed the description
requested review from @nik and removed review request for @hansegucker
added 1 commit
ed2721e3- Add missing templatesCompare with previous version
added 18 commits
master02c821f5- Merge branch 'master' into 331-user-registrationCompare with previous version
marked the checklist item LDAP account, person linked as completed
marked the checklist item Add social account from one of enabled providers as completed
marked the checklist item Register django-local account if preference enabled as completed
marked the checklist item Enable registration as completed
mentioned in merge request !2104
added 34 commits
master4c640a6c- Merge branch 'master' into 331-user-registrationCompare with previous version
added 1 commit
Compare with previous version
added 1 commit
28b14412- Clean up codeCompare with previous version
marked the checklist item Enable registration as incomplete
marked the checklist item LDAP account, person linked as incomplete
marked the checklist item Add social account from one of enabled providers as incomplete
marked the checklist item Register django-local account if preference enabled as incomplete
added 1 commit
f6c9da9d- Clean up codeCompare with previous version
added 7 commits
masterdd3aa077- Merge branch 'master' into 331-user-registrationCompare with previous version
added 1 commit
a50309a4- Fix templatesCompare with previous version
added 15 commits
master1af84364- Merge branch 'master' into 331-user-registrationCompare with previous version
changed this line in version 46 of the diff
added 5 commits
f49b0514- Allow users to change their password. Closes #354c2fbe569- Allow users to change their password. Closes #35484956b00- Merge branch '354-change-password' of edugit.org:AlekSIS/Official/AlekSIS-Core...38ec95c3- Merge branch '331-user-registration' into 354-change-password62a075ea- Merge branch '354-change-password' into '331-user-registration'Compare with previous version
added 3 commits
41c6e768- Fix templates2d841027- Merge branch '331-user-registration' into 353-password-resetc15e244b- Merge branch '353-password-reset' into '331-user-registration'Compare with previous version
mentioned in merge request AlekSIS-App-LDAP!1693
mentioned in issue #353
changed the description
changed the description
added 2 commits
28b14412- Clean up code1209a1df- Merge branch '331-user-registration' of...Compare with previous version
Is it possible to use the help text as label here? The extra label looks ugly in the frontend.
added 66 commits
master77b61fc9- Merge branch 'master' into 331-user-registrationCompare with previous version
changed this line in version 50 of the diff
changed this line in version 50 of the diff
added 3 commits
d0b8a870- Add password reset linkdd62ddec- Merge branch '331-user-registration' of...60a2b3d7- REwrite templatesCompare with previous version
resolved all threads
added 1 commit
91a78776- Use label instead of help_textCompare with previous version
added 5 commits
masterc33217b2- Merge branch 'master' into 331-user-registrationCompare with previous version
added 5 commits
master1de961c1- Merge branch 'master' into 331-user-registrationCompare with previous version
added 158 commits
master7882bb4c- Merge branch 'master' into 331-user-registrationCompare with previous version
added 24 commits
master6d5e9dac- Merge branch 'master' into 331-user-registrationCompare with previous version
added 25 commits
master2a851f31- Merge branch 'master' into 331-user-registrationCompare with previous version
It is possible to get some updated screenshots to document the final state of this MR?
added 10 commits
master2dba6aa3- Merge branch 'master' into 331-user-registrationCompare with previous version
added 1 commit
13b12eae- Fix help text in formCompare with previous version
added 43 commits
master35de37dc- Merge branch 'master' into 331-user-registrationdc5a9e65- Merge branch '331-user-registration' of...Compare with previous version
added 9 commits
master0c3778c8- Merge branch 'master' into 331-user-registrationCompare with previous version
added 15 commits
masterf8be13d3- Merge branch 'master' into 331-user-registrationCompare with previous version
added 43 commits
master9b00a0a1- Add tests8b810a8c- Merge branch 'master' into 331-user-registration0b3069be- Fix urlCompare with previous version
added 1 commit
Compare with previous version
added 1 commit
Compare with previous version
added 1 commit
08660b95- Add testsCompare with previous version
added 3 commits
7a4ffce2- Add custom account adapter8e9e3b90- Create person on signupCompare with previous version
added 1 commit
c7bac65e- Verify templatesCompare with previous version
added 45 commits
master794d6707- Merge branch 'master' into 331-user-registrationCompare with previous version
added 1 commit
8a1325fa- Fix menu iconCompare with previous version
changed the description
changed the description
added 1 commit
4743b747- Fix socialaccount settingsCompare with previous version
marked the checklist item Social accounts as incomplete
added 1 commit
c30a7996- Reformat templatesCompare with previous version
added 10 commits
masterb9ed1cb5- Merge branch 'master' into 331-user-registrationCompare with previous version
added 21 commits
master3f27351b- Merge branch 'master' into 331-user-registrationCompare with previous version
added 1 commit
8e21e8fe- Use custom change viewCompare with previous version
added 11 commits
master73247866- Merge branch 'master' into 331-user-registrationCompare with previous version
added 1 commit
01799e68- Fix disabling of password changeCompare with previous version
changed the description
marked the checklist item Disable changing of passwords as completed
Are social accounts still broken here? If so, we need it to be fixed quite immediately, @debdolph @hansegucker
added 34 commits
master9d068430- Merge branch 'master' into 331-user-registrationCompare with previous version
added 5 commits
masterfa4ec33f- Merge branch 'master' into 331-user-registrationf1ca5218- Update lockfileCompare with previous version
added 1 commit
Compare with previous version
added 1 commit
4122c5f8- Fix automated adding of social auth providers to INSTALLED_APPSCompare with previous version
added 1 commit
3e626237- Fix deletion of social accountsCompare with previous version
marked the checklist item Social accounts as completed
changed the description
added 2 commits
8ad6ab81- Update lockfile56073272- Fix code styleCompare with previous version
mentioned in commit
6ca32fa347