Skip to content

Remove UpsellPageComponent #19048

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

Merged
merged 11 commits into from
Jun 12, 2025
Merged

Remove UpsellPageComponent #19048

merged 11 commits into from
Jun 12, 2025

Conversation

oliverguenther
Copy link
Member

@oliverguenther oliverguenther commented May 28, 2025

Ticket

https://community.openproject.org/projects/gmbh/work_packages/64212/activity

What are you trying to accomplish?

  • Remove UpsellPageComponents with the new large banner variant.
  • Introduce a guard_enterprise_feature helper as discussed in the dev open door
    guard_enterprise_feature(:ldap_groups, except: %i[index]) do
      redirect_to action: :index
    end

Note: Not all pages have their upsell flow removed completely. For example, onedrive/sharepoint still refers to a separate landing page. We can discuss separately if a trial banner is required here, and how the page should behave like in this case.

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

@oliverguenther oliverguenther force-pushed the chore/remove-upsell-page branch 4 times, most recently from 8b74605 to 6a82acd Compare May 30, 2025 06:28
@oliverguenther oliverguenther force-pushed the chore/remove-upsell-page branch from 6a82acd to ca46d04 Compare May 30, 2025 07:12
@HDinger HDinger self-requested a review June 2, 2025 06:27
Copy link
Contributor

@HDinger HDinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good overall. The only issue I found is some inconsistnecy with regards to showing a PageHeader or not. I'd be in favour of showing the PageHeaders now. Those are the pages I found which are currently missing the PageHeader:

  • Notification/Date alerts
  • Notification/Sharing
  • OneDrive/SharePoint integration
  • PlaceholderUsers (see my other comment)
  • WP/Sharing queries (might be out of scope since they don't even have a pageHeader yet)

Further, the routes.rb file can be cleaned up a bit, now that we do not have the upsell routes any more.

@oliverguenther oliverguenther force-pushed the chore/remove-upsell-page branch 2 times, most recently from 6d26a67 to fe8b55d Compare June 10, 2025 06:55
@oliverguenther oliverguenther marked this pull request as ready for review June 10, 2025 06:55
@oliverguenther oliverguenther requested a review from HDinger June 10, 2025 06:55
@HDinger HDinger added this to the 16.2.x milestone Jun 12, 2025
@oliverguenther oliverguenther force-pushed the chore/remove-upsell-page branch from fe8b55d to a86fc24 Compare June 12, 2025 11:41
@oliverguenther oliverguenther force-pushed the chore/remove-upsell-page branch from a86fc24 to 9901050 Compare June 12, 2025 11:59
@oliverguenther oliverguenther merged commit f72bb94 into dev Jun 12, 2025
14 checks passed
@oliverguenther oliverguenther deleted the chore/remove-upsell-page branch June 12, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants