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

Update userDisplayNameField to use "displayName" instead of "gecos" #47

Merged
merged 1 commit into from
May 16, 2024

Conversation

stephdl
Copy link
Contributor

@stephdl stephdl commented May 15, 2024

This pull request updates the userDisplayNameField in the configure-module/20config file to use the "displayName" field instead of the "gecos" field.

However

  • when the user has already login, it does not honor the new setting because it has been written to postgres
webtop5=# SELECT * FROM core.users;
 domain_id  |   user_id    | type | enabled |               user_uid               |      display_name       |      se
cret      
------------+--------------+------+---------+--------------------------------------+-------------------------+--------
----------
 NethServer | john     | U    | t       | 24e4751b-fc50-4d01-8c65-107a98b7434a | de labrusse             | 3c6sm72
wdpuw22sg
 NethServer | foo       | U    | t       | 3beecb12-63e3-422f-b570-405277fd8cbf | de labrusse             | cqw7q6z
2ydn3i2es
 *          | admin        | U    | t       | 991f72dc-2b96-4340-b88f-53506b160519 | Administrator           | 76gvj7v
bpnuhsf6r
(7 rows)
  • when you save the configuration after the update of the module, it is not taken except if you change the user domain or the mail server

so in short it is valid only for first installation

NethServer/dev#6923

@stephdl stephdl requested review from Amygos and DavidePrincipi May 15, 2024 14:34
Copy link
Member

@DavidePrincipi DavidePrincipi left a comment

Choose a reason for hiding this comment

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

LGTM

@DavidePrincipi
Copy link
Member

@Amygos is this fix effective also for already provisioned instances?

@Amygos
Copy link
Member

Amygos commented May 16, 2024

@Amygos is this fix effective also for already provisioned instances?

No, the fix will apply only on fresh module installations, or when the value of mail_module and/ormail_domain changes, or after a module move/clone.
Also as pointed out by @stephdl the new value will be applied only for the users that log in for the first time.

To avoid the first issue we can resume the work in the PR #19, so we can always set the correct value in the configure-module action or on the module update.

For the users already created on db, I would leave the old value of the filed.

@stephdl
Copy link
Contributor Author

stephdl commented May 16, 2024

noobs question @Amygos, what is the purpose or the advantage of your solution ?

@stephdl stephdl merged commit da0d550 into main May 16, 2024
1 check passed
@stephdl stephdl deleted the sdl-6923 branch May 16, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants