Hi there,

The module does what it promises, but I was a bit surprised, that all the settings which is there is not being exported in configuration.
Such behaviour is unexpected and it is causing issues with deployment. Is there any reason why it is not in config? Is it possible it is a bug?

Tried it on D9.5.2. with PHP 8.1.

Thanks a lot.

CommentFileSizeAuthor
#12 domain_login_restrict-3336184-12.patch8.96 KBalexandre.todorov
#11 domain_login_restrict-3336184-11.patch8.98 KBalexandre.todorov
#10 domain_login_restrict-3336184-9.patch9 KBt_d_d
#7 domain_login_restrict-3336184-6.patch8.8 KBearthday47

Issue fork domain_login_restrict-3336184

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Petr Illek created an issue. See original summary.

sunilpawar’s picture

sunilpawar commented

We are storing the additional setting for the domain in a state like this.

function _domain_login_restrict_config_submit(&$form, FormStateInterface $form_state) {
  \Drupal::state()->set('domain_login_restrict_enabled', $form_state->getValue('domain_login_restrict_enabled'));
  \Drupal::state()->set('domain_login_restrict_assign_domain', $form_state->getValue('domain_login_restrict_assign_domain'));
}
sunilpawar’s picture

sunilpawar commented

@petr-illek

Do you have PATCH for it ?

thejimbirch’s picture

thejimbirch commented

I ran into this today. The configuration for the two settings is not exported.

Will try to find some tie to investigate.

earthday47’s picture

earthday47
Location New York
commented
Assigned: Unassigned » earthday47
Status: Active » Needs work

earthday47 opened merge request !6

earthday47’s picture

earthday47
Location New York
commented
Status: Needs work » Needs review
StatusFileSize
new domain_login_restrict-3336184-6.patch8.8 KB

I'm happy to help and supply a patch! I added MR 6 which converts State into configuration. I've also attached a patchfile to this comment.

Note that in this patch I've saved all the domain-specific configuration into the same domain_login_restrict.settings configuration object. Ideally, these config would be saved as 3rd party settings on the domain entities, but that's something that could be tackled in a future update.

thejimbirch’s picture

thejimbirch commented

Thanks for the patch Wes, and the knowledge that states are only saved in the database, not in config!

I will test this on my project next week!

t_d_d made their first commit to this issue’s fork.

t_d_d’s picture

t_d_d commented
Version: 8.x-1.1 » 8.x-1.2
StatusFileSize
new domain_login_restrict-3336184-9.patch9 KB

Added removal of state after config is created. New patch attached.

Note that when domain was previously created, had configuration saved in state and then was removed, its configuration is still saved in state and will remain there as there is no longer domain by which we can find that stale state now. Not sure how to solve this.

I guess it is the same for new config approach. It will too get stale when domain is removed later, but we can resolve that one in future when moving configuration to domain.

alexandre.todorov’s picture

alexandre.todorov commented
StatusFileSize
new domain_login_restrict-3336184-11.patch8.98 KB

Hello,

For those who want to apply this patch on the D11 compatibility commit, here is the rerolled patch on that commit 8bbdc8ffb

alexandre.todorov’s picture

alexandre.todorov commented
StatusFileSize
new domain_login_restrict-3336184-12.patch8.96 KB

And here is the rerolled version on the latest 8.x-1.x : e71777134

sunilpawar’s picture

sunilpawar commented

Can you update MR : https://git.drupalcode.org/project/domain_login_restrict/-/merge_requests/6 so i can merge it.