FreeUKGen / Management

Free REG issues not relating to product problems.
1 stars 0 forks source link

Restrict change of Transcription Agreement status to the individual, through the interface #35

Closed PatReynolds closed 6 years ago

PatReynolds commented 6 years ago

It appears that an individual can tick / untick on their ID page, and that their SC can do this - both need removing: only at sign-up and the existing transcriber interface can the individual tick (no one else should be able to).

PatReynolds commented 6 years ago

@Vino-S please ask if you need clarification.

Captainkirkdawson commented 6 years ago

on test2 and test3

Captainkirkdawson commented 6 years ago

@Vino-S There are significant concerns over this code change. 1) It permits the user to change their userid. Up to this time the userid once created has been considered as immutable. We adopted this initially to ensure that we could keep FR1 and FR2 in step. We could not afford to have it changed in 1 and not the other. This reason no longer exists. However the userid is not just a database object it is also the physical folder name for files uploaded by the user. That folder name is also used by the physical_files database collection and process and the attic database collection and process to store attic information for that user and their userid. The code as committed does not update the linkages in these other systems and therefore eliminates the linkage between the database and the underlying file system. It MUST not be deployed into production in its current form. We could add code to make those changes BUT we would also need to add recovery code to back out the changes in the event that any one part of the update system fails. 2) The change as coded now eliminates the ability of the SC or manager to make ANY changes to the user's profile including role, syndicate as well as disable them. There are many functions that a manager must be able to do to a profile. So the code needs to be tailored to only affect the the one field transcriber agreement. All attempts to make are not permitted with the message XXXXXXXXXXX : Userid already exists which in itself is incorrect.

Vino-S commented 6 years ago

Thanks @Captainkirkdawson for letting me know about missing the update of changed userid in physical files and attic files and also for disabling the change of userid field.I was not aware of that, I agree that we need additional code changes and will start working on it immediately.

Captainkirkdawson commented 6 years ago

@Vino-S Before you do that we need an urgent discussion on permitting a userid change. The whole structure of FR was based on the userid being immutable. It is the sole link between the physical file infrastructure on the servers and the logical structure of the database. The possibility of unintended consequences of such a change are high and their impact potentially catastrophic. @SteveBiggs @edickens @richpomfret @PatReynolds @benwbrum What is the requirement behind making the userid editable?? I have never seen or heard of a requirement.

edickens commented 6 years ago

Not from me. I agree it should never be changed. A duplicate cannot be set up, and if someone has mistyped and prefers something else, just create the new one and get the SC to delete the one that is unwanted. This has never happened anyway.

SteveBiggs commented 6 years ago

I have no idea either.

SteveBiggs commented 6 years ago

I've just noticed that the Last email confirmation field is manually editable too (and also is on the live site) which seems strange to me:

image

Vino-S commented 6 years ago

@Captainkirkdawson It was one of Pat's suggestion while I was working on transcription agreement and importing users for FreeCEN. I will send an email to Pat to get a clear requirement.

Captainkirkdawson commented 6 years ago

That is a weak requirement for such a fundamental change IMO.

Kirk Dawson 5220 Riverside Drive Fairmont Hot Springs, B.C. V0B 1L1

On 21 February 2018 at 08:28, Vino-S notifications@github.com wrote:

@Captainkirkdawson https://github.com/captainkirkdawson It was one of Pat's suggestion while I was working on transcription agreement and importing users for FreeCEN. I will send an email to Pat to get a clear requirement.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/FreeUKGen/FreeREGNonProductIssues/issues/35#issuecomment-367363238, or mute the thread https://github.com/notifications/unsubscribe-auth/ACvdRvg3-2B0uIZgZHf0QY4Lc5njJ5V5ks5tXDYGgaJpZM4Q_OfE .

Vino-S commented 6 years ago

I will revert the changes and re-work on this issue, if needed, once Pat returns

Captainkirkdawson commented 6 years ago

Personally I would see nothing wrong with the task being completed according to the open data requirement. Just don't add in the ability to change the userid until we have agreed that it is required. That change has nothing to do with open data. As I noted in my second point above the code as implemented stops the SC or anyone else from making ANY change to a profile not just the Transcriber agreement field

Captainkirkdawson commented 6 years ago

@Vino-S see the proposed integration of our 2 pieces of work into the edit form in #1310.

The issue on the SC changes is not with the form it is with the code https://github.com/FreeUKGen/MyopicVicar/blob/Issue_35/app/controllers/userid_details_controller.rb#L545-L572 Since the code says there is a userid change any SC change of anyfield will fall to https://github.com/FreeUKGen/MyopicVicar/blob/Issue_35/app/controllers/userid_details_controller.rb#L567-L568 which is the message they get

Captainkirkdawson commented 6 years ago

Vino has removed option to change userid and code redeployed