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

Expose installation status to the Pontoon server #139

Merged

Conversation

MikkCZ
Copy link
Owner

@MikkCZ MikkCZ commented Apr 18, 2021

#138

How to detect Pontoon Add-on in Pontoon frontend:

  1. Create a listener
    window.addEventListener('message', (e) => {
      const data = JSON.parse(e.data);
      if (data._type === 'PontoonAddonInfo') {
        const pontoonAddonInfo = data.value;
        ...
      }
    });
    to act when Pontoon Add-on is installed or executes the hook after the page is loaded.
  2. When loading, check window if the info is already present, which happens if Pontoon Add-on is already installed and executes the hook before the page is loaded.
    const pontoonAddonInfo = window.PontoonAddon;
    ...
  3. In both cases the object is of the same type.
    interface PontoonAddonInfo {
      installed: boolean;
    }
    Currently there is only a single field which is always true, however in the future it may allow for extensibility and/or letting Pontoon know if the extension is being disabled on uninstalled.

TODO:

  • honor Pontoon server base URL to inject/register the content script
  • use Pontoon server base URL to secure window.postMessage

@MikkCZ MikkCZ requested a review from mathjazz April 18, 2021 11:45
@codecov
Copy link

codecov bot commented Apr 18, 2021

Codecov Report

Merging #139 (2d29abb) into master (2d727fa) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #139   +/-   ##
=======================================
  Coverage   93.26%   93.26%           
=======================================
  Files          37       37           
  Lines         401      401           
  Branches       87       87           
=======================================
  Hits          374      374           
  Misses         26       26           
  Partials        1        1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d727fa...2d29abb. Read the comment docs.

@mathjazz
Copy link
Collaborator

Thanks, Michal!

I've installed the add-on as you suggested in the bug and I'm able to access the window.PontoonAddon on localhost:8000 (or production server, depending on the configuration).

I'm however unable to make the add-on popup work when used with the local Pontoon instance (http://localhost:8000). It says "There was an error fetching data from Pontoon. Please check, if you are signed in", but I am signed in. It works fine with prod. I don't think I'll need that to verify your patch regarding bug 1705242 requirements, but I'd be happy to learn more about the issue and how to debug it.

@MikkCZ
Copy link
Owner Author

MikkCZ commented Apr 19, 2021

I'm however unable to make the add-on popup work when used with the local Pontoon instance (http://localhost:8000). It says "There was an error fetching data from Pontoon. Please check, if you are signed in", but I am signed in. It works fine with prod. I don't think I'll need that to verify your patch regarding bug 1705242 requirements, but I'd be happy to learn more about the issue and how to debug it.

That's interesting. I must admin I haven't pointed the extensions for a Pontoon instance running locally for a while. The error message itself it just a hint to sign in, it's shown when fetching the notifications fails for whatever reason. Did you by any change sign in to the local Pontoon in a container (like tab container inside Firefox)? If not, I have more free time tomorrow and I will try reproduce this issue, as well as finish the PR.

@mathjazz
Copy link
Collaborator

Did you by any change sign in to the local Pontoon in a container (like tab container inside Firefox)?

Nope.

@MikkCZ
Copy link
Owner Author

MikkCZ commented Apr 22, 2021

Sorry for the delay. Feel free to use the current version as is for the promotion. I have a busy week and will try to solve the notifications issue over weekend.

@mathjazz
Copy link
Collaborator

mathjazz commented Apr 22, 2021

Thanks for the update.

The patch seems to be doing what it's supposed to do. I wonder if we should ship it as is and address the remaining bits in a followup. That would give users some time to update before we start rolling out the promotion (which will be also shown to the users of the old add-on version).

Thoughts?

@MikkCZ
Copy link
Owner Author

MikkCZ commented Apr 22, 2021

I created a follow up ticket for the localhost issue, which is really unrelated to this particular change.

What is the ETA for the promotion banner in Pontoon? The update process is pretty fast and shouldn't take more than a couple of days for everyone to get the update. Personally, I would wait till the weekend, when I have time to address the last TODO and release it completed. Anyway if I do not manage to address it, I can merge and release on Sunday and fix it later. Or do you want to speed up the release even more?

@mathjazz
Copy link
Collaborator

Release on Sunday (or Monday) sounds good.

We plan to start the promotion early in May (May 1 or May 3).

@MikkCZ MikkCZ changed the title [WIP] Expose installation status to the Pontoon server Expose installation status to the Pontoon server Apr 25, 2021
@MikkCZ
Copy link
Owner Author

MikkCZ commented Apr 25, 2021

Hi @mathjazz. This PR is ready for review. Please review it including the initial comment, where I describe how this integration would work. If all looks good, we can merge it and proceed to release then.

Copy link
Collaborator

@mathjazz mathjazz left a comment

Choose a reason for hiding this comment

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

Nice work submitting this patch on such a short notice!

Also thanks for the detailed installation and usage instructions.

@MikkCZ MikkCZ merged commit 6d7ac9e into master Apr 26, 2021
@MikkCZ MikkCZ deleted the pontoon-addon-promotion-expose-itself-to-pontoon-server branch April 26, 2021 19:23
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.

None yet

2 participants