-
Notifications
You must be signed in to change notification settings - Fork 30
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 143 fix escrow decimal #299
Open
hamede-abdulgafur
wants to merge
13
commits into
involveMINT:main
Choose a base branch
from
hamede-abdulgafur:feat-143-fix-escrow-decimal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Feat 143 fix escrow decimal #299
hamede-abdulgafur
wants to merge
13
commits into
involveMINT:main
from
hamede-abdulgafur:feat-143-fix-escrow-decimal
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ck-from-gitignore Removed package-lock.json from .gitignore
This PR adds a file environments.org.ts file to the .gitignore file. This file will be referenced with the new way to execute the application locally when someone is a part of the organization. We do this by adding another configuration to the angular.json file for an "org" environment. In addition, two new scripts were added to the package.json file to work with this.
Updated readme to include steps on how to configure Firebase, launch PostgreSQL in a container, and start both client and server.
These updates allow for us to use the npm run deploy:test to deploy both the api and the ui to the test envrionment. I also updated the emailing and other services to treat test like prod and allow for notifications to be sent. ---------
Updates as suggested via codelyzer and depcheck to fix issue with npm restore failing. --------- Co-authored-by: Quinn Heffern <[email protected]>
…T#132) I added a pgpass and servers.json file to the .docker folder to be used in the docker-compose.yml file. These allow us to setup the pgadmin container to automatically add an "Involvemint Local" server with access to the postgres container so that new engineers do not have to worry about adding the server or authenticating with the local involvemin database in order to view it from pgadmin. Updated the init.sql scrip to successfully create the involvemin database when first running the docker compose up command. The previous script failed due to the "if not exists" part. Updated the build process locally to use the local flag instead of the org flag. This is more logical from an engineers perspective. It means that the environment file will be environment.local.ts and the commands to run the server and client are npm run start:server:local and npm run start:client:local respectively. I updated the readme to reflect the changes made in this branch.
Updated the docker compose to use 5433 as the locally accessible port in order to not clash with any locally running postgres instances outside of the docker container. We were running into issues where if someone already has a local postgres server running, the application was connecting to it instead of the docker container. This is fine as long as the developer knows where to look, but for getting DFG running as quick as possible this eliminates a bit of confusion.
Updates to use a default address when running locally instead of depending on access to the geolocation api. The default address ensures that the default latitude and longitude match the addresses. Adds docker container that host the firebase emulator suite so that we can run the application without using a gcp storage bucket.
…tors to run successfully (involveMINT#292)
justpenguins
approved these changes
Jun 7, 2024
fogunsan
requested changes
Jun 7, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the commits made are not your own. You can create another branch checkout from dfg/sum24-involvement, and then add your changes to that branch. Adjust your PR to merge onto dfg/sum24-involvemint not main. Also upload a video of you testing the code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug Description
The bug involved incorrect decimal formatting for credits in escrow within the wallet section of the application. Instead of displaying the correct value with two decimal places (e.g., 90.00), the application was showing the credits in escrow as whole numbers without decimals (e.g., 9000).
Root Cause
The issue was caused by the escrowBalance value being handled as a whole number (in cents) without converting it to a formatted decimal value (dollars).
code changes
this.updateState({
credits,
creditsLoaded: loaded,
balance: balance / 100, // Correctly format the balance
escrowCredits,
escrowBalance: escrowBalance / 100, // Correctly format the escrow balance
});