Skip to content

Sachin/barcode scan async issue#507

Open
imsks wants to merge 25 commits into
developfrom
sachin/barcode-async-issue
Open

Sachin/barcode scan async issue#507
imsks wants to merge 25 commits into
developfrom
sachin/barcode-async-issue

Conversation

@imsks

@imsks imsks commented Sep 8, 2021

Copy link
Copy Markdown
  1. Fixed Barcode scan issues
  2. Added Dexie to store data into Index DB
  3. Created API Queue table to call APIs in the background

@imsks imsks requested a review from with-shrey September 8, 2021 12:26
Comment thread temp.txt Outdated
@@ -0,0 +1,4 @@
sudo docker-compose logs -f admin

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is this file ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed it.

Comment thread notification-service/package-lock.json Outdated
"lockfileVersion": 1,
"requires": true,
"dependencies": {
"@sentry/apm": {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dont commit files you haven't worked on

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please remove these changes

Check changes in source tree before committing

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Okay.

Comment thread docker-compose.yml Outdated
image: nginx:stable
ports:
- "80:80"
- "81:80"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Revert this to default

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread admin/src/environments/environment.ts Outdated
production: false,
API_URL: '/api',
BASE_URL: '',
BASE_URL: 'https://stockup-staging.shoppinpal.com',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

revert this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

import { Injectable } from "@angular/core";

// Declare Database
class ProductDatabase extends Dexie {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why two different database classes?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ProductDatabase is for storing products & APIQueueDatabase is for API queing

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if (!searchedOrderItem.received) {
this.notReceivedLineItems = [searchedOrderItem];
} else {
async scanBarcodeAPI(sku?: string, force: boolean = false) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

duplicate code
this code already exists in barcode service

Can we reuse it ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes. Fixed it.

import Dexie from "dexie";

// Declare Database
class ProductDatabase extends Dexie {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we have two database classes ?
Please use I class/database to contain both tables

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Both table have different functions and that's why it'll be confusing to use a single class

super("APIQueueDatabase");

this.version(1).stores({
skus: "++id, sku",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

table name skus doesn't make sense

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It'll only hold skus in rows to call the APIs


export interface BarcodeReceive {
sku: string;
done: boolean;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I dont see any use of done argument in this file ?

Why we have less parameters in skus table but the type has more parameters ?
Please add the missing parameters in skus table definition in db class

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We don't need any more data in skus. It works expected now

@@ -0,0 +1,33 @@
import Dexie from "dexie";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is another file that does the same thing
indexdb.service.ts

Please delete this file and use the angular service instead

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deleted

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.

2 participants