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

feat(design): view donation page (punch list) #291

Merged
merged 3 commits into from
Jan 4, 2022
Merged

Conversation

pattishin
Copy link
Collaborator

@pattishin pattishin commented Dec 22, 2021

Related to task: #288

Emblem punch list: https://docs.google.com/document/d/1lfO6xWmqfpqa1q4XlbbRmzUpEAaeWg9wMQ0MwrMxL-c/edit?usp=sharing

Items to verify (from above master list)

  • GET /donation displays a form with formatting problems (title Campaign Name and value overlap). The form should also include the donor's name and address.
  • POST /donation should update the donor's name and address, if needed.
  • GET /viewDonation isn't formatted, and lacks the time of the donation. Also shows the amount to one decimal place instead of two.

Steps to test PR through my local project:

If you go this route, let me know so I can add you manually as an approver in the project. (Required in order to login)
I also may need to redeploy.

api: https://api-byccf5bkda-uc.a.run.app
website: https://website-byccf5bkda-uc.a.run.app/

Steps to test PR locally in your personal project: (Cloud Shell)

  1. Ensure you have your local setup like so:
  1. Copy client-libs/ directory from root into website/ directory before building the image / deploying (its Dockerfile requires the directory)

  2. Build image based on content-api/ directory and deploy in container w/ Cloud Run (name: api)

  3. Build image based on website/ directory and deploy in container w/ Cloud Run (name: website)

  4. From project root, run . scripts/configure_auth.sh (Requires website container to exist)

  5. Verify the above listed checklist items.

Drive of personal scripts used:
https://drive.google.com/drive/folders/1knOSYk4EKQcuke5wvCMVVB-ifJN7JgNO?usp=sharing

refactor: minor templating fix
…tamp ref

refactor: minor fix, moving address/name input to be updated under donor

refactor: cleaning up
@pattishin pattishin requested a review from a team as a code owner December 22, 2021 18:55
@pattishin pattishin linked an issue Dec 22, 2021 that may be closed by this pull request
3 tasks
@pattishin pattishin self-assigned this Dec 22, 2021
refactor: reformatting main_nonapprover_test
Copy link
Contributor

@engelke engelke left a comment

Choose a reason for hiding this comment

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

LGTM with comments:

1 - I don't know why the error on the API tests occurred because I don't seem to have permission to view the test results. It's going to need to be fixed - let's see how to get the needed permission.

2 - For now, it's okay to fill in the mailing address as a dictionary, but it would be better to format it as multiple lines, as in:

123 Main St.
Anytown, WA 99999

@grayside
Copy link
Collaborator

grayside commented Jan 4, 2022

I'm also have trouble viewing that result. It looks to me like this is the most recent test run: https://console.cloud.google.com/cloud-build/builds;region=global/bac39fad-fd51-40c3-a6f0-c3bf51bd073f?project=emblem-ops (and is passing).

@grayside
Copy link
Collaborator

grayside commented Jan 4, 2022

Rerunning, it failed again and I don't see the build in Cloud Console.

@grayside
Copy link
Collaborator

grayside commented Jan 4, 2022

@dinagraves noted we have redundant triggers in place, from a test on spinning up a new Emblem instance that reused our main repository. These checks were not deleted after the test. The check is still showing as failed, but Dina has cleaned up the underlying bits and the check is no longer blocking merge. Proceed when ready.

@engelke engelke merged commit 9b3adb6 into main Jan 4, 2022
@engelke engelke deleted the feat/view-donation-page branch January 5, 2022 17:21
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.

feat(design): view donation page (punch list)
3 participants