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: added data table for transactions of saving account #1844

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

PC-11-00
Copy link
Collaborator

Description

Now we can add the data table for transactions entity type of saving account and also we can edit,delete and create the datatable.

Related issues and discussion

#1726

Screenshots, if any

transaction_datatable.mov

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • If you have multiple commits please combine them into one commit by squashing them.

  • Read and understood the contribution guidelines at web-app/.github/CONTRIBUTING.md.

Now we can add the data table for transactions entity type of saving account and also we can edit,delete and create the datatable.

Fixes: openMF#1726
@bharathcgowda
Copy link
Collaborator

@ramvr1256 @luckyman20 could you please help in reviewing this PR?

Copy link
Contributor

@luckyman20 luckyman20 left a comment

Choose a reason for hiding this comment

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

Please keep an eye on how you comment. The logic seems fine.

@bharathcgowda I hope this is tested.

Tests are failing as well, please see to that.

import { Injectable } from '@angular/core';
import { Resolve, ActivatedRouteSnapshot } from '@angular/router';

//rxjs Imports
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//rxjs Imports
// rxjs Imports

import { Injectable } from '@angular/core';
import { Resolve, ActivatedRouteSnapshot } from '@angular/router';

//rxjs Imports
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//rxjs Imports
// rxjs Imports

/**
*
* @param route
* @returns {Observable<any>}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @returns {Observable<any>}
* @returns {Observable<boolean>}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually below should be any

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please elaborate?

})

export class DatatableTransactionTabComponent implements OnInit {
entityId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Need comment for this variable as well.

});
}

ngOnInit(): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it required if it is empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it is not required I can remove it, actually when we generate a component in angular this comes automatically.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can remove it too.

children:[{
path:':datatableName',
component:DatatableTransactionTabComponent,
data: {title : extract('View Data table'),routeParamBreadcrumb:'datatableName'},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
data: {title : extract('View Data table'),routeParamBreadcrumb:'datatableName'},
data: {title : extract('View Data table'), routeParamBreadcrumb:'datatableName'},

path: 'datatables',
children:[{
path:':datatableName',
component:DatatableTransactionTabComponent,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
component:DatatableTransactionTabComponent,
component: DatatableTransactionTabComponent,

{
path: 'datatables',
children:[{
path:':datatableName',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
path:':datatableName',
path: ':datatableName',

@@ -6,6 +6,7 @@ export const appTableData: { displayValue: string, value: string }[] = [
{ displayValue: 'Loan Account', value: 'm_loan' },
{ displayValue: 'Saving Account', value: 'm_savings_account' },
{ displayValue: 'Loan Product', value: 'm_product_loan' },
{displayValue: 'Saving Account Transaction',value: 'm_savings_account_transaction'},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{displayValue: 'Saving Account Transaction',value: 'm_savings_account_transaction'},
{ displayValue: 'Saving Account Transaction', value: 'm_savings_account_transaction' },


/**
*
* @param savingsService
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @param savingsService
* @param savingsService Savings Service.

@PC-11-00
Copy link
Collaborator Author

@luckyman20 which tests are failing?

@luckyman20
Copy link
Contributor

@luckyman20 which tests are failing?

I can see that task named 'Publish Image in Docker Hub / build (pull_request) has failed.'

@PC-11-00
Copy link
Collaborator Author

@luckyman20 docker hub test failing will not affect the code when it will be deployed, it will work fine docker hub is failing because it can't able to log in, I don't have the right to log in to the docker hub

Copy link
Contributor

@luckyman20 luckyman20 left a comment

Choose a reason for hiding this comment

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

LGTM!

@PC-11-00
Copy link
Collaborator Author

@bharathcgowda please merge it now.

@bharathcgowda
Copy link
Collaborator

Thank you @luckyman20 for reviewing this properly and thank you @PC-11-00 for fixing all the comments.

@bharathcgowda bharathcgowda merged commit f557088 into openMF:master Aug 18, 2023
1 of 2 checks passed
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.

3 participants