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

Add setting to restrict sidebar sections to local only #435

Merged
merged 10 commits into from
Jan 17, 2024

Conversation

thepaperpilot
Copy link
Contributor

When enabled by an admin, this will make the random magazines, active people, random posts, and random threads only show the local items of those types. This does not affect the sections when looking at a specific community.

@e-five256
Copy link
Member

This is very minor, but it seems like we have been going with naming things mbin instead of kbin when adding new things, but haven't quite tackled the older env vars as things like the .env copy usually happen only once for instances and we don't want to break already running instance owners, but it'd be nice if all the places that kbin was added could be replaced with mbin

@thepaperpilot
Copy link
Contributor Author

This is very minor, but it seems like we have been going with naming things mbin instead of kbin when adding new things, but haven't quite tackled the older env vars as things like the .env copy usually happen only once for instances and we don't want to break already running instance owners, but it'd be nice if all the places that kbin was added could be replaced with mbin

To clarify, you're asking for new env vars to start with MBIN_ but leave the KBIN_ ones in tact, so we have a mix of both?

@e-five256
Copy link
Member

e-five256 commented Jan 15, 2024

To clarify, you're asking for new env vars to start with MBIN_ but leave the KBIN_ ones in tact, so we have a mix of both?

Yea, that's what I ended up doing when adding the default_theme one most recently https://github.com/MbinOrg/mbin/blob/2bf67ceed54c889672c4e932b9fbfd6769a78de9/.env.example#L32C1-L33 (I agree it's confusing, but the other options have their cons as well. Probably not appropriate to add code that says kbin, and we can't easily switch everything that exists to mbin without lots of defensive fallback coding having to be added first)

@thepaperpilot
Copy link
Contributor Author

Just a head's up, I still plan on addressing the feedback on this PR but before that I'd like to change the behavior. I'd rather the random post and threads were from local magazines, regardless of whether the user was local.

.env.example Outdated Show resolved Hide resolved
@e-five256
Copy link
Member

e-five256 commented Jan 16, 2024

aside from my one final comment this appears fine to me, but I'm no expert, @melroy89 @nobodyatroot not sure if you two or anyone else has opinions, I tested this locally and not having the env var set doesn't cause any issues (I was kind of expecting it to need default: in the services.yaml but... it seems fine? so shouldn't be an issue for current instance owners), can set the admin setting and it adds the db setting, removing it sets the value to false in db, and setting the env var as true has the admin setting checked, so all looked right to me, I just can't quite tell if the filters are correct because all my content is local so... it is just as local as it always was

thepaperpilot and others added 2 commits January 16, 2024 17:41
Co-authored-by: e-five <146029455+e-five256@users.noreply.github.com>
@e-five256 e-five256 merged commit 34ed5c3 into MbinOrg:main Jan 17, 2024
7 checks passed
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.

None yet

2 participants