NERC-CEH / fit-count-app

Fit Count App & Website
Apache License 2.0
0 stars 0 forks source link

Remove first name and last name fields #141

Closed kazlauskis closed 1 year ago

kazlauskis commented 2 years ago

Follow up from #139.

The website already has a field_full_name. @andrewvanbreda

andrewvanbreda commented 2 years ago

Hi @kazlauskis

Have thought about this a bit. If we don't enable easy_login, I think we have the trouble that if the user updates their account on the website their details such as name it won't update on the Warehouse.

I am not sure what other implications not having easy_login enabled would have.

I am a little unsure of the current workflow. What code is actually creating the Indicia Warehouse account, is it standard Indicia code? I would have thought the most natural workflow would be for the app to create a Drupal account and that fires the easy_login hooks which creates a Warehouse user account, otherwise there is danger of bypassing elements of code that are part of standard user create.

Hi @johnvanbreda What are the implications of running an Indicia website without easy_login enabled? The Drupal account won't sync to the Warehouse will it? Will Indicia data entry forms even work?

Cheers

johnvanbreda commented 2 years ago

@andrewvanbreda you'd need to test it, but in theory everything will still work in the recording forms as long as the user profile has a field for indicia_user_id. The easy login module is responsible for filling this value in so you will need to recreate that code. As the warehouse expects a name to be split into first name and surname there may be complexities though.

The easy login module also has some other functionality for you to consider:

  1. Allow the user to set a preferred recording location and taxon groups (not necessary)
  2. GDPR handling of account deletion
  3. Syncing any user profile fields to their equivalent on the warehouse (also not necessary?)

What about altering Easy Login so that if you hide the last name and first name fields (making sure they aren't mandatory) it uses the field_full_name field instead if it's present (with some clever logic to separate last from first name). Would need to explain this in some documentation somewhere though...

andrewvanbreda commented 2 years ago

Hi @johnvanbreda @kazlauskis Thanks for thoughts. My original proposal in https://github.com/NERC-CEH/fit-count-app/issues/139 was to create a new module which is essentially bare minimum easy_login, so that would solve the problem of altering existing easy_login. That module would have GDPR account deletion handling on it. Your points 1 and 3 probably won't be necessary in this case. I am concerned that updating details on the account such as the user's name now won't sync to the warehouse though.

Guess I am going to have to get testing on this one.

kazlauskis commented 2 years ago

This site includes the drupal-8-module-indicia-api module that automatically sets the Indicia user ID to the profile. It uses a user_insert and user_presave hooks to check if the ID is missing and if so then fetches it. The full name is split in a very simplified way at the moment.

andrewvanbreda commented 2 years ago

@kazlauskis Thanks for the info. Will examine this situation.

andrewvanbreda commented 1 year ago

Hi @kazlauskis @johnvanbreda

I have given this a test now, and created an "Easy login account deletion only" module on my machine.

I think my overall summary is, this should work mostly, but is not as elegant as when full easy_login is used.

WORKING WELL

LIMITATIONS

Karolis, does the FIT Count APP have user account management on it? If so, does updating the details on it reflect in the indicia People table?

I am not really sure how much this matters. They will go out of sync, but whether anyone actually thinks it matters that this happens. Once we have concluded our discussion I will check with David.

I also found it a little tricky to uninstall the Easy Login module once installed, but it is doable I think. I had problems with it interacting with the Options module, and causing an error on the user page. I was able to resolve this easily enough on my machine, but am assuming that FIT Count Website will behave same way. I think it depends a lot of fields in use etc, so I can't guarantee it will be the same difficulty.

@BirenRathod Is in ok for me to experiment on the FIT Count Test site, and then on Live, and these sites are def recoverable in the event of problems?

johnvanbreda commented 1 year ago

@andrewvanbreda I don't think the people table gets updated by Easy Login either. This is an area we need to review - but because the changes can't be automatically reflected back into other Drupal sites the account syncing is limited.

andrewvanbreda commented 1 year ago

@johnvanbreda It does, but I think it is very particular about when it does it. For instance, the email address will update if they have only one website account. So that is my thinking, that it doesn't matter too much if FIT Count doesn't at all if it only does it sometimes anyway.

I guess we can probably go ahead. Will wait for Biren to respond.

johnvanbreda commented 1 year ago

I agree. Just note that this area is something I plan to review so the main Easy Login functionality might be updated.

kazlauskis commented 1 year ago

Karolis, does the FIT Count APP have user account management on it? If so, does updating the details on it reflect in the indicia People table?

No, it doesn't. The app allows you to register, but that's about it.

andrewvanbreda commented 1 year ago

@kazlauskis Fit Count Live is now running a slim easy login module (easy_login_account_deletion_only). I removed the first name and last name fields which still seemed to be present after easy_login was removed.. I checked this worked on my machine, but might be worth check that account deletion still fires the anonymiser. So that would be to delete an account, and then we observe that the asscociated email address either receives an email that they are a member of other sites, or in the case where their are no sites, we would see its email address is anonymised. It should work though, worked fine on mine.

@johnvanbreda I have written small drupla two modules relating to all of this

  1. A tiny module that stops the accoun Cancel link being shown to non admins (admin_only_account_cancel)
  2. The slimmed down easy login which only contains the anonymiser and is suitable for projects where only a fullname is used (easy_login_account_deletion_only).

Let me know if you want either of these in Indicia Features in the main Indicia repository, else will just leave them in the pantheon project responsitories.

andrewvanbreda commented 1 year ago

@kazlauskis Even if the anonymiser isn't tested, def worth you checking app hasn't been affected in general use like it was before. I did get biren to take a FIT Count site backup in case of any issues.

kazlauskis commented 1 year ago

@andrewvanbreda the app works fine 👍 Please close the ticket if your part is done.

andrewvanbreda commented 1 year ago

Hi @kazlauskis Just realised I can't close, as I don't have sufficient rights. Can you close, I will email John separately about where he wants the commits. Cheers.

kazlauskis commented 1 year ago

I sent you an invite now to join the repo maintainers.

andrewvanbreda commented 1 year ago

@kazlauskis Thanks, accepted :)