Last pre-launch checks
Closed this issue · 7 comments
AS A Bank.Green staff member
I WANT the devs to check off a bunch of things before switching the API to data.bank.green
SO THAT users aren't negatively impacted by the switch
Things to check for
- create brands via data.bank.green/admin
- edit brands via data.bank.green/admin
- edit brand commentary via data.bank.green/admin
- "Top three ethical" commentary button results in banks being recommended on the sustainable banks page
- "Recommended Order" results in appropriate changes to the bank recommendation order on the sustainable banks page (feature was removed because unused – Simon)
- Edits of "headline" and other fields are reflected on individual bank pages
- Only banks currently shown on the live site are shown on the test.bank.green site
Acceptance criteria
- Each of the boxes are investigated
- For each thing not working, the issue is either fixed or a ticket is created to fix it
Changes in progress in the pre-launch-checks branch
Hey @shuesken
- data.bank.green is currently running the pre-launch-checks branch
- PR #51's recent changes to the backend broke frontend's query, which is making it hard to test the remaining checkboxes in the front end. Would you please fix that?
- It seems like the frontend query, as designed, is hitting 3 different tables. Would it be enough to make one query just to get the bank names by country and then load the bank info later on, or does the frontend need all of that data initially for SPA stuff?
Would you please be able to fix this?
@RogerTangos
I've updated https://test.bank.green with the correct query.
I'm not sure I understand your point about hitting multiple models – the query includes two variables you might have overlooked, withCommentary
and withFeatures
, that make the inline directive for those properties conditional. They're only set when requesting detail information for banks or when looking at the top banks. The graphql backend should correctly parse those variables and not return information from the Commentary and Features model unless requested, and as far as I can tell, that works:
@RogerTangos I've made a few more fixes for things as I discovered them in testing. As far as I'm concerned, things look good. I do not know how to properly test the last requirement – as far as I'm aware, we show all banks that exist in the airtable, and only those we import. Maybe you have a better intuition on what spots to check here? I do think the PR has grown quite large by now though and could use another review.
The last checkbox I tested just by counting the number of li
elements in the list of banks from the united states. In the test site,
document.getElementById('foolist').getElementsByTagName("li").length // test site
>> 100
document.getElementById('foolist').getElementsByTagName("li").length // live site
>> 140
It seems that graphene limits results to 100, and I haven't figured out how to remove this limitation yet. Currently reviewing the rest of the PR, @shuesken
@RogerTangos I fixed the limit in settings.py
with GRAPHENE = {"RELAY_CONNECTION_MAX_LIMIT": 10000}
Thank you.