search

Tychobra / polished

Authentication and Administration for Shiny apps
https://polished.tech
Other
234 stars 36 forks source link

added app_name if in the polished yaml but not the configs, #169

Closed muschellij2 closed 2 years ago

muschellij2 commented 2 years ago

This is a common error I would make and actually made, so may be a high likelihood of others? Just doing the first 2 as these are the most commonly used args.

muschellij2 commented 2 years ago

Could generalize this with using a modifyList here, but higher likelihood of conflict or wrong argument names. Could simply do:

args = c(
    "app_name",
    "api_key",
    "firebase_config",
    "admin_mode",
    "is_invite_required",
    "sign_in_providers",
    "is_email_verification_required",
    "is_auth_required",
    "sentry_dsn",
    "cookie_expires"
  )
global_sessions_config_args = global_sessions_config_args[args]

or more generally:

global_sessions_config_args = global_sessions_config_args[formals(polished::global_sessions_config)]

or this depending on how you construct global_sessions_config_args

global_sessions_config_args = global_sessions_config_args[intersect(names(global_sessions_config_args), formals(polished::global_sessions_config))]

but that gets tricky if you later introduce ...

Overall, I think the first 2 are sufficient, but may be a good help out for users with giving them flexibility to forget the global_sessions_config: YAML header, but not being too flexible to make it a huge lift of maintenance IMO

merlinoa commented 2 years ago

Yes I like it. I'll go ahead and merge. I also like the formals approach 2 for subsetting the arguments. I don't like the "...", argument, so I'm not worried about "..." becoming an issue later. I prefer to just pass a list to and argument names ".dots" if you want to include arbitrary arguments rather than using "...".

As a side note, I have been thinking about changing the name of "global_sessions_config" to something easier to remember. I think gloabl_session_config is a pretty bad function name. Maybe something like "secure_global", "global_config", or "polished_config". Let me know if you agree and if you have a suggestion for the new name.

muschellij2 commented 2 years ago

Polished config and then alias global sessions config with deprecation warning for 2-3 versions

On Sat, Dec 18, 2021 at 2:11 PM Andy Merlino @.***> wrote:

Merged #169 https://github.com/Tychobra/polished/pull/169 into master.

— Reply to this email directly, view it on GitHub https://github.com/Tychobra/polished/pull/169#event-5788388639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIGPLXWUZJCBYC2LSI6JDLURTME7ANCNFSM5KK6KAFQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

-- Best, John