verdnatura/salix
verdnatura
/
salix
3
4
Fork 0
Code Issues Pull Requests 67 Releases Wiki Activity

3977-invoiceIn.tax #969

Merged
joan merged 6 commits from 3977-invoiceIn.tax into dev 2022-05-13 13:04:44 +00:00
Conversation 0 Commits 6 Files Changed 7 +141 -15
vicent commented 2022-05-09 12:51:57 +00:00
Contributor
Copy Link
No description provided.
vicent added the
CR / Tests passed
Simple
 labels 2022-05-09 12:51:57 +00:00
vicent added 2 commits 2022-05-09 12:51:59 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
a6b8674302 feat(invoiceIn.tax): add button for create an expence
gitea/salix/pipeline/head There was a failure building this commit Details
f08a384a7c fix: e2e test
joan added 1 commit 2022-05-11 12:47:04 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
afe4f0ba94 Merge branch 'dev' into 3977-invoiceIn.tax
carlosjr added 1 commit 2022-05-11 13:37:46 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
c749438f32 Merge branch 'dev' into 3977-invoiceIn.tax
carlosjr requested changes 2022-05-11 14:02:31 +00:00
modules/invoiceIn/front/tax/index.html Outdated
@ -35,1 +35,4 @@
<tpl-item>{{id}}: {{name}}</tpl-item>
<append>
<vn-icon-button
vn-tooltip="Create expence"
carlosjr commented 2022-05-11 13:43:59 +00:00
Contributor
Copy Link

you have a typo in expence, it is expense

you have a typo in expence, it is expense
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.html Outdated
@ -100,0 +110,4 @@
vn-id="createExpense"
on-accept="$ctrl.onResponse()">
<tpl-body>
<section class="SMSDialog">
carlosjr commented 2022-05-11 13:48:03 +00:00
Contributor
Copy Link

SMSdialog?

SMSdialog?
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.html Outdated
@ -100,0 +117,4 @@
vn-id="code"
label="Code"
ng-model="$ctrl.expence.code"
required="true">
carlosjr commented 2022-05-11 13:48:49 +00:00
Contributor
Copy Link

shouldn't this one be the focused input?

shouldn't this one be the focused input?
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.js Outdated
@ -28,1 +29,4 @@
}
onResponse() {
if (!this.expence)
carlosjr commented 2022-05-11 13:45:47 +00:00
Contributor
Copy Link

expense

expense
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.js
@ -29,0 +31,4 @@
onResponse() {
if (!this.expence)
throw new UserError(`The fields can't be empty`);
else if (!this.expence.code)
carlosjr commented 2022-05-11 13:50:28 +00:00
Contributor
Copy Link

if the code inside the conditional throws, there's no point using "else if"

if the code inside the conditional throws, there's no point using "else if"
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.js Outdated
@ -29,0 +36,4 @@
else if (!this.expence.description)
throw new UserError(`The description can't be empty`);
const params = [];
carlosjr commented 2022-05-11 13:53:38 +00:00
Contributor
Copy Link

if you are creating a data entity to send on a query, why would you call it params and why woulnd't you defined it as follows:

const data = [{
id: this.expence.code,
isWithheld: this.expence.isWithheld,
name: this.expence.description
}]

if you are creating a data entity to send on a query, why would you call it params and why woulnd't you defined it as follows: const data = [{ id: this.expence.code, isWithheld: this.expence.isWithheld, name: this.expence.description }]
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.spec.js
@ -56,2 +59,4 @@
});
});
describe('onResponse()', () => {
carlosjr commented 2022-05-11 14:00:05 +00:00
Contributor
Copy Link

there's only 1 test for the onResponse function but there are about 4 use cases for this function, 3 thrown errors and un success query. Some tests are missing

there's only 1 test for the onResponse function but there are about 4 use cases for this function, 3 thrown errors and un success query. Some tests are missing
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/index.spec.js
@ -58,0 +61,4 @@
describe('onResponse()', () => {
it('should return success message', () => {
controller.expence = {
carlosjr commented 2022-05-11 13:45:58 +00:00
Contributor
Copy Link

expense

expense
vicent marked this conversation as resolved
modules/invoiceIn/front/tax/locale/es.yml Outdated
@ -0,0 +1,6 @@
Create expence: Crear gasto
carlosjr commented 2022-05-11 13:46:05 +00:00
Contributor
Copy Link

expense

expense
vicent marked this conversation as resolved
carlosjr removed the
Simple
CR / Tests passed
 labels 2022-05-11 14:03:14 +00:00
vicent added 1 commit 2022-05-12 06:27:16 +00:00
gitea/salix/pipeline/head There was a failure building this commit Details
568579f461 refactor: pull request changes
vicent requested review from carlosjr 2022-05-12 06:27:33 +00:00
vicent added the
CR / Tests passed
 label 2022-05-12 06:27:50 +00:00
joan added 1 commit 2022-05-13 12:59:37 +00:00
gitea/salix/pipeline/head This commit looks good Details
913b76880a Merge branch 'dev' into 3977-invoiceIn.tax
joan approved these changes 2022-05-13 13:04:35 +00:00
joan left a comment
Contributor
Copy Link

LGTM

LGTM
joan merged commit fdd780a470 into dev 2022-05-13 13:04:44 +00:00
joan deleted branch 3977-invoiceIn.tax 2022-05-13 13:04:45 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
carlosjr
joan
Labels
Clear labels   
CR / Tests passed

Unit tests and e2e passed

  
e2e

Implement e2e

  
Missing tests

Missing tests

  
Needs refactor

   
Simple

Simple check / No CR needed

No Label
CR / Tests passed
e2e
Missing tests
Needs refactor
Simple
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: verdnatura/salix#969
No description provided.
Delete Branch "3977-invoiceIn.tax"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?