Skip to content

Email subscription improvements#1756

Merged
Oaphi merged 38 commits intodevelopfrom
0valt/1351/subscriptions
Aug 6, 2025
Merged

Email subscription improvements#1756
Oaphi merged 38 commits intodevelopfrom
0valt/1351/subscriptions

Conversation

@Oaphi
Copy link
Copy Markdown
Member

@Oaphi Oaphi commented Aug 5, 2025

closes #1351

With this change applied:

  • we no longer redirect to 404 when directly accessing /subscriptions/new;
  • filled in subscription fields are preserved between submission attempts;
  • it becomes possible to change subscription type / qualifier on the fly;
  • it also makes possible to change qualifiers on the fly with dynamic category, tag, and user selects;
  • users with JavaScript disabled can complete the whole flow;

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.27%. Comparing base (35b5554) to head (7e21641).
⚠️ Report is 46 commits behind head on develop.

Additional details and impacted files
Components Coverage Δ
controllers 69.86% <100.00%> (+0.10%) ⬆️
helpers 79.20% <100.00%> (+0.18%) ⬆️
jobs 48.57% <ø> (ø)
models 86.12% <100.00%> (+0.20%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Oaphi Oaphi requested review from a team, ArtOfCode- and cellio August 5, 2025 18:41
@cellio

This comment was marked as outdated.

@Oaphi

This comment was marked as outdated.

@Oaphi

This comment was marked as outdated.

Comment thread app/views/subscriptions/new.html.erb Outdated
@cellio
Copy link
Copy Markdown
Member

cellio commented Aug 5, 2025

As for qualifier, for tags it's prefilled (good) and for posts it's irrelevant but can be edited. It doesn't look like editing it does anything

I did at least try to warn users in the field's description - do you think it's not noticeable enough? Maybe we should reword it until we are able to provide JS-based reactivity?

How about: "tag name for tag subscriptions, user number for user subscriptions, otherwise leave blank"? (The main point we want to convey is that if there's nothing here already, don't touch it, but that would be a little blunt.)

Also, I forgot to test user subscriptions before; my comment about the tag name being in the first field applies for user names too.

Comment thread app/helpers/subscriptions_helper.rb Outdated
@Oaphi
Copy link
Copy Markdown
Member Author

Oaphi commented Aug 5, 2025

How about: "tag name for tag subscriptions, user number for user subscriptions, otherwise leave blank"? (The main point we want to convey is that if there's nothing here already, don't touch it, but that would be a little blunt.)

Agreed on something like that (not sure if that's going to be needed in the first place with the latest updates), except for the "user number", I'd much rather we refer to ids as "ids"

@Oaphi Oaphi changed the title 0valt/1351/subscriptions Email subscription improvements Aug 5, 2025
@Oaphi Oaphi force-pushed the 0valt/1351/subscriptions branch from b9d2204 to deff5bb Compare August 5, 2025 22:26
@cellio
Copy link
Copy Markdown
Member

cellio commented Aug 6, 2025

UI changes look good!

I see that category subscriptions are on this list, but I don't think we have a UI affordance for them. I think maybe we did a long, long time ago before we added RSS? Anyone know the status of these (do they work today and need a UI affordance?)?

@Oaphi
Copy link
Copy Markdown
Member Author

Oaphi commented Aug 6, 2025

Anyone know the status of these (do they work today and need a UI affordance?)?

I don't think there's any (we should probably add them back) - I haven't been able to find links for categories similar to other types of subscriptions

@Oaphi
Copy link
Copy Markdown
Member Author

Oaphi commented Aug 6, 2025

Phew, the updated flow now works like this:

ssr-2025-08-06_20.10.51.mp4

Added fully functioning type-specific selects for qualified subscriptions instead.
Really didn't want this to become another big update, but oh well, should be worth the effort.

@Oaphi
Copy link
Copy Markdown
Member Author

Oaphi commented Aug 6, 2025

Users with JavaScript disabled will now always see a simple text field with an explanation to still be able to create subscriptions. I believe this is an acceptable trade-off (we can't initialize searchable selects without JS):

Screenshot from 2025-08-06 20-25-56

@Oaphi Oaphi requested a review from ArtOfCode- August 6, 2025 17:40
@Oaphi Oaphi added this to the v0.12.3 milestone Aug 6, 2025
@Oaphi Oaphi merged commit 4153f8d into develop Aug 6, 2025
13 checks passed
@Oaphi Oaphi deleted the 0valt/1351/subscriptions branch August 6, 2025 19:29
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.

Subscribing from a subscription error page produces a 404

3 participants