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

generalize profile links #1524

Open
wants to merge 56 commits into
base: develop
Choose a base branch
from
Open

Conversation

cellio
Copy link
Member

@cellio cellio commented Jan 26, 2025

Fixes #1248.

Adds user-controlled extra fields to the profile to replace the baked-in website and twitter slots. You can put whatever you want here; proper URLs get linkified and everything else is text, like so:

Edit profile section:

editor UI

Main profile view:

profile rendering

The same link-nerfing rules that applied before apply to this new section.

Added a maintenance task to initialize new table rows for all current users. After discussion, we decided this would be better than constructing new rows on demand (and creating the "add" UI in the editor). When the task is run, all users will get three entries in user_websites, with the first two being filled with the website and twitter links if they exist. This allows the profile editor to present a form with three label/URL pairs for editing. For new users, we add these rows after creation.

Out of scope here (could be considered in the future):

  • After this safely deploys without issues, delete the old website/twitter values after migrating. (TBD: maintenance task to remove them or schema change to drop the columns.) Remove from authenticated_user_object and configure_permitted_parameters as well (mentioned in chat by Oaphi).
  • Make the number of websites configurable in a global site setting. Handle the potential data loss if the limit is lowered (the main reason we're not doing this now). My recommendation would be to preserve the data and limit the number that are shown. (In the editor, we'd show the extras in some grayed-out-like state, so that users could still recover that data if they wanted to move it up.)
  • Better integration w/third-party platforms, for example generating the icon and link like we did for twitter before this change. (I'm concerned about maintenance burden and complexity here, but acknowledge that entering a proper link vs. just a username could be a stumbling block for some users.)

Deployment note: After deploying, visit /maintenance_tasks, click on the initialize task (the only one), then click on "run". With a large number of users we might need to do that more than once, so check the output. (I believe the task times out after 5 minutes.)

@cellio cellio marked this pull request as draft January 26, 2025 19:21
app/controllers/users_controller.rb Outdated Show resolved Hide resolved
app/controllers/users_controller.rb Outdated Show resolved Hide resolved
app/models/user_website.rb Outdated Show resolved Hide resolved
app/views/users/edit_profile.html.erb Outdated Show resolved Hide resolved
app/views/users/show.html.erb Outdated Show resolved Hide resolved
@Oaphi Oaphi force-pushed the cellio/1248-generalize-profile-links branch from 648b3ad to c85a227 Compare January 29, 2025 06:10
@Oaphi Oaphi force-pushed the cellio/1248-generalize-profile-links branch from c85a227 to acaa348 Compare January 29, 2025 06:10
@cellio cellio marked this pull request as ready for review January 31, 2025 02:15
@cellio cellio requested a review from ArtOfCode- January 31, 2025 16:10
app/controllers/users_controller.rb Outdated Show resolved Hide resolved
app/views/users/show.html.erb Outdated Show resolved Hide resolved
app/models/user.rb Outdated Show resolved Hide resolved
test/controllers/users_controller_test.rb Outdated Show resolved Hide resolved
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.

Generalize external links on user profile
3 participants