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

Fix "See full details" link for the RRM module #9144

Closed
1 task done
kelvinballoo opened this issue Aug 6, 2024 · 5 comments
Closed
1 task done

Fix "See full details" link for the RRM module #9144

kelvinballoo opened this issue Aug 6, 2024 · 5 comments
Labels
Module: RRM Reader Revenue Manager module related issues P0 High priority Team M Issues for Squad 2 Type: Bug Something isn't working

Comments

@kelvinballoo
Copy link
Collaborator

kelvinballoo commented Aug 6, 2024

Bug Description

When I click on the '[See full details in Reader Revenue Manager]' link on RRM module in the view mode, I get routed to a page hitting 400 error.
The error page is attached.

Steps to reproduce

  1. Activate RRM
  2. Go to the RRM module under view mode

Screenshots

Click on [See full details in Reader Revenue Manager]' link
Screenshot 2024-08-06 at 11 36 50

400 error appears:
Screenshot 2024-08-06 at 11 36 55

Additional Context

  • PHP Version: 8.0
  • OS: Mac OS Sonoma
  • Browser: Chrome
  • Plugin Version: 1.133.0
  • Device: Macbook Pro

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The "See full details in Reader Revenue Manager" link on the RRM module page should open the following URL in a new tab:
    • https://publishercenter.google.com

Implementation Brief

  • Update the homepage value for the RRM module to be https://publishercenter.google.com.

'homepage' => __( 'https://readerrevenue.withgoogle.com/', 'google-site-kit' ),

Test Coverage

  • Update any failing tests.

QA Brief

  1. Activate RRM
  2. Go to the RRM module under view mode.
  3. Click on "See full details in Reader Revenue Manager" link. It should direct to https://publishercenter.google.com/ in a new tab. Note that there might be an extra parameter pli which is fine.

Changelog entry

  • Update the "See full details" link in Reader Revenue Manager.
@wpdarren wpdarren added Type: Bug Something isn't working Module: RRM Reader Revenue Manager module related issues labels Aug 6, 2024
@techanvil techanvil assigned techanvil and unassigned techanvil Aug 6, 2024
@techanvil techanvil changed the title 400 error on RRM module full details link Fix "See full details" link for the RRM module Aug 6, 2024
@ivonac4 ivonac4 added the Team M Issues for Squad 2 label Aug 6, 2024
@nfmohit nfmohit added the P0 High priority label Aug 6, 2024
@nfmohit
Copy link
Collaborator

nfmohit commented Aug 6, 2024

IB ✅

@ankitrox ankitrox self-assigned this Aug 7, 2024
@ankitrox ankitrox removed their assignment Aug 7, 2024
@nfmohit nfmohit assigned nfmohit and tofumatt and unassigned nfmohit Aug 7, 2024
@tofumatt tofumatt removed their assignment Aug 12, 2024
@kelvinballoo
Copy link
Collaborator Author

QA Update ⚠️

@ankitrox @nfmohit , I have a suggestion for the link.
I am thinking if we should append the publication ID as well, based on what is selected in RRM module so that when we click the [See full details in Reader Revenue Manager]' link, we are directed to the correct Publication.

The reason why is because if a user has multiple publications, the publication they might see might not be the correct one.

Refer to the video below.

publication.redirect.mov

@techanvil
Copy link
Collaborator

Hi @kelvinballoo, thanks for the suggestion. I had considered this too while drafting the AC. However, we have a standard pattern of linking to the statically defined homepage for the service in these "See full details" links.

For example, in the Analytics settings view, the "See full details in Analytics" link points to https://analytics.google.com; we provide the "Edit in Analytics" link to take the user directly to the connected property.

image

A similar pattern can be seen in Tag Manager, AdSense etc.:
image

image

So, the consistent thing to do here is to maintain the current target of https://publishercenter.google.com for the "See full details" link, but we should consider adding an "Edit in Reader Revenue Manager" link or suchlike to take the user directly to the publication...

@nfmohit, has this already come up for consideration for the RRM module?

@kelvinballoo
Copy link
Collaborator Author

QA Update ✅

Thanks @techanvil . Agreed on that.
With that, this ticket is QA pass. We are no longer hitting 400 error page.

Successful.linkage.mov

@nfmohit , this is a QA pass and can be moved to approval but since there is a follow up question for you here, I am assigning this ticket to you first.

@nfmohit
Copy link
Collaborator

nfmohit commented Aug 13, 2024

@nfmohit, has this already come up for consideration for the RRM module?

Unfortunately, no, it looks like we missed this during the design doc/UX design stage. Thank you for pointing it out. I have just created a new issue #9192 that adds this.

@nfmohit , this is a QA pass and can be moved to approval but since there is a follow up question for you here, I am assigning this ticket to you first.

I've opened a follow-up issue. As this is a QA pass, I'm moving it to Approval, thanks @kelvinballoo !

@nfmohit nfmohit removed their assignment Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: RRM Reader Revenue Manager module related issues P0 High priority Team M Issues for Squad 2 Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants