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

refactor(efb): Restructured API files #7984

Merged
merged 14 commits into from
Jun 1, 2023

Conversation

MicahBCode
Copy link
Contributor

@MicahBCode MicahBCode commented Apr 23, 2023

Summary of Changes

This PR has changed the folder structure of the APIs used for the EFB (e.g. Navigraph) and bundles them together in a separate folder. The Navigraphs Authentication/Authorization has been outsourced into the new folder structure to make it reusable for new integrations across the EFB.

Additional context

This PR is part of the preparation for the PR #7702 .
In addition this is a part of the preceeding PR #7728 that caused problems for a few users. The rest of the PR will be redone and submitted in a new PR.

Discord username (if different from GitHub): Micah#3002

Testing instructions

Please test the following functions:

  • The Navigraph Authentication/Authorization is still working as intended.
  • The Charts Integration still works as before the update.
  • The Simbrief Integration still works as before the structural changes were made.

How to download the PR for QA

Every new commit to this PR will cause a new A32NX artifact to be created, built, and uploaded.

  1. Make sure you are signed in to GitHub
  2. Click on the Checks tab on the PR
  3. On the left side, click on the bottom PR tab
  4. Click on the A32NX download link at the bottom of the page

@MicahBCode MicahBCode marked this pull request as ready for review April 25, 2023 19:49
@github-actions github-actions bot added this to 🟡 Code Review: Ready for Review in Quality Assurance Apr 25, 2023
@2hwk 2hwk added this to the v0.11.0 milestone Apr 27, 2023
Quality Assurance automation moved this from 🟡 Code Review: Ready for Review to 🟣 QA Team Review: Ready to Test Apr 27, 2023
@2hwk 2hwk added the QA Tier 1 label Apr 27, 2023
@alepouna
Copy link
Member

alepouna commented May 18, 2023

CS/Contributor/Unofficial Test Report

Discord : Alepouna🌙#9824
Object of testing: #7984
Tier of Testing : 1
Date : 18/05/2023

Testing Process:

  • Open EFB
  • Import (and auto-import) Simbrief profile
  • Attempt authentication & charts

Negatives:

  • Could not use Navigraph Authentication/Authorization (infinite loading)
    image
  • Reset Navigraph Authentication inop
  • Could not use charts integration due to above

Testing Results:
Not passed

Notes:
Mayhap a key is required for the authentication to work?

@MicahBCode
Copy link
Contributor Author

CS/Contributor/Unofficial Test Report

Discord : Alepouna🌙#9824 Object of testing: #7984 Tier of Testing : 1 Date : 18/05/2023

Testing Process:

  • Open EFB
  • Import (and auto-import) Simbrief profile
  • Attempt authentication & charts

Negatives:

  • Could not use Navigraph Authentication/Authorization (infinite loading)
    image
  • Reset Navigraph Authentication inop
  • Could not use charts integration due to above

Testing Results: Not passed

Notes: Mayhap a key is required for the authentication to work?

This is most likely to be a PR issue with the API keys used. They need to be stored securely and should never be visible. Therefore testing might have to be done using a local build by someone of the FBW Team with access to those secrets instead.

@tracernz
Copy link
Member

You shouldn't get this far if the API keys are not present. Instead the page just shows "insufficient env" or something to that effect.

@MicahBCode
Copy link
Contributor Author

You shouldn't get this far if the API keys are not present. Instead the page just shows "insufficient env" or something to that effect.

You don't because the check for the ENV variables only checks if they are not present at all. But as I can see in the build process in the Github Actions for the PR builds it is written but as an empty string. Therefore this happens.

@Benjozork
Copy link
Member

Dev Team Test Report

Object of testing: #7984
Tier of Testing : 1
Date : 01/06/2023

Testing Process:

With client secret

  • Wake up EFB
  • Test SimBrief import -✅
  • Test authn and charts loading - works - ✅

Without client secret (empty string)

  • Wake up EFB
  • Test authn and charts loading - Insufficient .env file - ✅

Negatives:
None

Testing Results:
Passed

@Benjozork Benjozork merged commit 1081ae1 into flybywiresim:master Jun 1, 2023
5 checks passed
Quality Assurance automation moved this from 🟣 QA Team Review: Ready to Test to ✔️ Done Jun 1, 2023
@MicahBCode MicahBCode deleted the efb-apis-structure branch July 16, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants