Skip to content

Landing Page preview not available unless user is logged in #14766

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Mar 27, 2025
Merged

Landing Page preview not available unless user is logged in #14766

merged 6 commits into from
Mar 27, 2025

Conversation

dadarya0
Copy link
Contributor

@dadarya0 dadarya0 commented Mar 20, 2025 •
edited
Loading

MAUT-5511 - Landing Page preview not available unless user is logged in

Q A
Bug fix? (use the a.b branch) ❌
New feature/enhancement? (use the a.x branch) ✔️
Deprecations? ❌
BC breaks? (use the c.x branch) ❌
Automated tests included? ✔️
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

The users are not able to preview the landing pages unless they have user accounts.

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Create a landing page.
  3. There should be a toggle in preview mode to make public
  4. Open the preview and copy the URL.
  5. Paste the URL in an incognito/private window or other browser where you aren't logged into Campaign Studio to open the preview.
  6. if public preview mode enabled then preview should be available otherwise access permission error should be displayed

@dadarya0
Merge pull request mautic#1333 from acquia/MAUT-5511
d98f294 
MAUT-5511 - Landing Page preview not available unless user is logged in
dadarya0
dadarya0 commented Mar 20, 2025
View reviewed changes
@codecov Codecov
Copy link

codecov bot commented Mar 20, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.04%. Comparing base (867fe53) to head (fce71b0).
⚠️ Report is 1937 commits behind head on 7.x.

Files with missing lines Patch % Lines
app/bundles/PageBundle/Entity/Page.php 76.47% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                7.x   #14766   +/-   ##
=========================================
  Coverage     65.03%   65.04%           
- Complexity    34843    34851    +8     
=========================================
  Files          2290     2290           
  Lines        103917   103929   +12     
=========================================
+ Hits          67586    67601   +15     
+ Misses        36331    36328    -3
Files with missing lines Coverage Δ
...bundles/PageBundle/Controller/PublicController.php 72.95% <100.00%> (+0.07%) ⬆️
app/bundles/PageBundle/Entity/Page.php 93.75% <76.47%> (-1.28%) ⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dadarya0 dadarya0 added the unforking Used for PRs in the Acquia's unforking initiative label Mar 20, 2025
@dadarya0 dadarya0 requested review from a team, rohitpavaskar and nileshlohar and removed request for a team March 20, 2025 06:57
nileshlohar
nileshlohar approved these changes Mar 20, 2025
View reviewed changes
Copy link
Contributor

@nileshlohar nileshlohar left a comment

Choose a reason for hiding this comment

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

Code changes LGTM.

@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged landing-pages Anything related to landing pages code-review-passed PRs which have passed code review labels Mar 23, 2025
imaabasiee
imaabasiee reviewed Mar 24, 2025
View reviewed changes
Copy link

@imaabasiee imaabasiee left a comment

Choose a reason for hiding this comment

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

Tested this pr but got lost at step 6 @dadarya0 . But here is a screenshot of what I got when I pasted the preview URL in an incognito browser.
image

@dadarya0
Copy link
Contributor Author

Tested this pr but got lost at step 6 @dadarya0 . But here is a screenshot of what I got when I pasted the preview URL in an incognito browser. image

if public preview mode is enabled (toggle is green) then it will display page content otherwise will show error.
image

@imaabasiee please check public preview is enabled or disabled.

Bastian2718
Bastian2718 approved these changes Mar 26, 2025
View reviewed changes
Copy link

@Bastian2718 Bastian2718 left a comment

Choose a reason for hiding this comment

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

LGTM!

escopecz
escopecz requested changes Mar 26, 2025
View reviewed changes
Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

A few suggestions for more modern, typed code

imaabasiee
imaabasiee approved these changes Mar 26, 2025
View reviewed changes
Copy link

@imaabasiee imaabasiee left a comment

Choose a reason for hiding this comment

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

When public preview is on:
Screenshot 2025-03-26 162253

When public preview is off:
Screenshot 2025-03-26 162310

@nileshlohar nileshlohar requested a review from escopecz March 27, 2025 08:20
escopecz
escopecz requested changes Mar 27, 2025
View reviewed changes
escopecz
escopecz approved these changes Mar 27, 2025
View reviewed changes
Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

The code looks good. Thanks!

@escopecz escopecz added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people. and removed pending-test-confirmation PR's that require one test before they can be merged labels Mar 27, 2025
@escopecz escopecz added this to the 7.0.0-alpha milestone Mar 27, 2025
@escopecz escopecz merged commit bab6d45 into mautic:7.x Mar 27, 2025
15 checks passed
@escopecz escopecz added the enhancement Any improvement to an existing feature or functionality label Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RCheesley RCheesley RCheesley left review comments

@escopecz escopecz escopecz approved these changes

@rohitpavaskar rohitpavaskar Awaiting requested review from rohitpavaskar rohitpavaskar was automatically assigned from mautic/acquia-engineers

+3 more reviewers

@nileshlohar nileshlohar nileshlohar approved these changes

@imaabasiee imaabasiee imaabasiee approved these changes

@Bastian2718 Bastian2718 Bastian2718 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality landing-pages Anything related to landing pages ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged unforking Used for PRs in the Acquia's unforking initiative user-testing-passed PRs which have been successfully tested by the required number of people.
Projects
None yet
Milestone
7.0.0-alpha
Development

Successfully merging this pull request may close these issues.
6 participants
@dadarya0 @escopecz @RCheesley @nileshlohar @imaabasiee @Bastian2718