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

Update staging by syncing with upstream #5

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from

Conversation

DavidTheProgrammer
Copy link

@DavidTheProgrammer DavidTheProgrammer commented Oct 7, 2024

Description

This PR updates our version of the Wazimap-ng backend with the latest changes from the upstream repo. It also maintains the addition of the display_format field in the ProfileKeyMetrics model that is used internally and resolves all conflicts arising from that change.
Other changes are in regards to migrations, the PR merges some diverging leaves of root migrations following the addition of our custom field as explained above.
Some tests are failing and this is because they are failing even on the upstream branch, I've fixed some of them that were failing due to the addition of the display_format field and other easy to catch errors. Considering opening a PR to push the changes upstream??
View commits for full changes after conflict resolution, including the fixing of some of the failing test files.

Related Issue

#4

How to test it locally

Run the docker compose command as shown in the README.md and verify it's working as expected.

Changelog

Added

  • AWS_QUERYSTRING_AUTH to readme and updated it's location to the main settings file. Changed read mode from os.environ.get to env.str to align with upstream.

Updated

  • Location of DISPLAY_FORMAT_CHOICES constant to align with upstream changes of moving it to constants.py file.
  • Resolved conflicts in multiple files resulting from the addition of the display_format field.

Removed

  • Additional settings files to align with upstream changes.

Checklist

  • 🚀 is the code ready to be merged and go live?
  • 🛠 does it work (build) locally

Pull Request

  • 📰 good title
  • 📝good description
  • 🔖 issue linked
  • 📖 changelog filled out

Commits

  • commits are clean
  • commit messages are clean

Code Quality

  • 🚧 no commented out code
  • 🖨 no unnecessary logging
  • 🎱 no magic numbers
  • black was run locally (as part of the pre-commit hook)

Testing

  • ✅ added (appropriate) unit tests
  • 💢 edge cases in tests were considered
  • ✅ ran tests locally & are passing - Mostly

@DavidTheProgrammer DavidTheProgrammer self-assigned this Oct 7, 2024
@DavidTheProgrammer DavidTheProgrammer marked this pull request as ready for review October 8, 2024 09:12
@DavidTheProgrammer DavidTheProgrammer requested a review from a team October 8, 2024 09:13
@kilemensi
Copy link
Member

👍🏽 @DavidTheProgrammer.

Have we tested this against our FE apps (climatemappedafrica and pesayetu) and all works well?

@DavidTheProgrammer
Copy link
Author

👍🏽 @DavidTheProgrammer.

Have we tested this against our FE apps (climatemappedafrica and pesayetu) and all works well?

I tested it against climatemappedafrica but not pesayetu. Let me try it now, but from all indications I don't expect any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review | Staging
Development

Successfully merging this pull request may close these issues.

2 participants