Skip to content

Log items that share the same UUID #53

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.

Sign up for GitHub

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

Open
Spazcool wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Log items that share the same UUID #53

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9790b58
REFACTOR: logScanned to catch doubled UUIDs & render doubles on template
Dec 14, 2017
f19fcc8
FIX: hacked out bug
Dec 19, 2017
4aea460
REMOVE: cut duplication in logScanned func
Dec 19, 2017
e4bb3c8
REFACTOR: logScanned works again
Dec 19, 2017
b6dc388
REMOVE: double UUID conditional from recent.ejs
Dec 19, 2017
93da255
ADD: double UUID conditional without duplicate code
Dec 19, 2017
cae791d
ADD: double UUID conditional without duplicate code
Dec 19, 2017
5b37912
ADD: link to item row in spreadsheet to both item obj and recent.ejs
Dec 19, 2017
6f7a97f
Refactoring/simplifying code googleSpreadsheet.js
mpsido Dec 20, 2017
1c1d78a
Removing hard url in app.js
mpsido Dec 20, 2017
d04c2cf
Refactoring, addRecentlyScanned function instead of logScanned
mpsido Dec 20, 2017
9d8e1c8
Merge master
mpsido Dec 29, 2017
12229de
Fixing pre-commit lint warnings, and adding addQrImg function to items
mpsido Dec 29, 2017
3cd5020
moving filterEmptyUuid and sortByFloor functions to attribute of matc…
mpsido Dec 29, 2017
d33e8d9
Removing deprecated similar items function
mpsido Dec 29, 2017
9b0b9a6
Fixing qrList regression
mpsido Dec 29, 2017
e773185
Reintroducing cellRef
mpsido Dec 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,26 @@ const { loadDatabase, searchDatabase } = require('./googleSpreadsheet');

const app = express();

const allScans = [];
const allScans = new Map();

Copy link
Member

@lenzai lenzai Dec 20, 2017

Choose a reason for hiding this comment

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

why?

Copy link
Contributor

@mpsido mpsido Dec 21, 2017

Choose a reason for hiding this comment

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

Because I want to avoid repetitions in the "recent" page.
I want to add one by one unique items on that page

Copy link
Member

@lenzai lenzai Dec 22, 2017

Choose a reason for hiding this comment

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

This is not the way to handle changes and features. We create an issue, discuss it and push it.
Hidding secret features in unrelated commits is not appropriate decision workflow and distracting for code review

function logScanned(uuid, matches) {
let allMatches = matches;
const now = new Date();
function addRecentlyScanned(uuid, item, nbFound = 0) {
// TRICK: only record uuids
if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(uuid)) {
return;
}
allScans.map(item => item.status = (item.uuid === uuid) ? 'fixed' : item.status);
if (allMatches.length > 1) {
allMatches = allMatches.splice(0, 1);
allMatches[0].double = true;

const duplicatedItem = item;

Copy link
Member

@lenzai lenzai Dec 20, 2017

Choose a reason for hiding this comment

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

duplicatedItem is redundant with item we don't need 2 names for 1 thing.

Copy link
Contributor

@mpsido mpsido Dec 21, 2017

Choose a reason for hiding this comment

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

I don't know if item is a reference or a copy.
I wanted to ensure it is a copy because I am writing in it.

Copy link
Member

@lenzai lenzai Dec 21, 2017

Choose a reason for hiding this comment

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

I don't think this is creating any copy.

Copy link
Contributor

@mpsido mpsido Dec 21, 2017

Choose a reason for hiding this comment

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

I need to recheck that, but if that is a reference, then I need the local variable

Copy link
Member

@lenzai lenzai Dec 22, 2017

Choose a reason for hiding this comment

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

not really useful reply

duplicatedItem.time = new Date();
duplicatedItem.duplicated = nbFound > 1;

if (nbFound === 0) {
duplicatedItem.fixture = '';
duplicatedItem.uuid = uuid;
duplicatedItem.status = 'missing';
}
allScans.unshift({
time: now,
fixture: allMatches[0].fixture ? allMatches[0].fixture : '',
status: allMatches[0].fixture ? '' : 'missing',
uuid,
double: allMatches[0].double,
link: allMatches[0].cellRef,
});

allScans.set(uuid, duplicatedItem);
}

app.set('view engine', 'ejs');
Expand Down Expand Up @@ -67,17 +65,17 @@ app.get('/:uuid', (req, res) => {
loadDatabase((allItems) => {
const matches = searchDatabase(req.params, allItems);
if (matches.length === 0) {
logScanned(req.params.uuid, [{ fixture: null }]);
addRecentlyScanned(req.params.uuid, {});
res.status(404).render('notFound', {
item: '',
id: req.params.uuid,
});
return;
}
logScanned(req.params.uuid, matches);
if (matches.length >= 1) {
if (matches.length > 1) {
console.log(`Too much matches for uuid ${req.params.uuid} length = ${matches.length}`);
}
addRecentlyScanned(req.params.uuid, matches[0], matches.length);
matches[0].similarItems = searchDatabase({ fixture: matches[0].fixture }, allItems)
Copy link
Member

@lenzai lenzai Dec 20, 2017

Choose a reason for hiding this comment

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

off topic

Copy link
Author

@Spazcool Spazcool Dec 20, 2017

Choose a reason for hiding this comment

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

Perhaps amateur questions, but why remove the hard URL and how else do we get a link to work?

.filter(item => item.uuid !== matches[0].uuid)
.splice(0, 3);
Expand Down
18 changes: 9 additions & 9 deletions views/recent.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
<h1>Recently scanned QR codes</h1>
<article class="col-xs-12">
<ul class='recent-list'>
<% for (var i = 0; i < allScans.length; i++) { %>
<% for (let item of allScans.values()) { %>
<li>
Copy link
Member

@lenzai lenzai Dec 20, 2017

Choose a reason for hiding this comment

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

@mpsido nice! Are there other areas in the code where this rewrite should be applied?

Copy link
Contributor

@mpsido mpsido Dec 21, 2017

Choose a reason for hiding this comment

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

Everywhere there is a for loop ;) if you don't care about the index then you can use for..in or for..of.
See the MSDN documentation to understand the difference ;)

Copy link
Member

@lenzai lenzai Dec 22, 2017

Choose a reason for hiding this comment

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

I mean do it.

<%- allScans[i].time.toTimeString() %> -
<% if (allScans[i].fixture) { %>
<%- allScans[i].fixture %>
<%- item.time.toTimeString() %> -
<% if (item.fixture) { %>
<%- item.fixture %>
<% } else { %>
???
<% } %>
: <br/>
<a href="/<%- allScans[i].uuid %>"><%- allScans[i].uuid %></a>
<% if (!allScans[i].fixture) { %>
<a href="<%- allScans[i].link %>" style="color: <%- allScans[i].status === 'fixed' ? 'orange' : 'red' %>">Missing Details!</a>
<a href="/<%- item.uuid %>"><%- item.uuid %></a>
<% if (!item.fixture) { %>
<a href="<%- item.link %>" style="color: <%- item.status === 'fixed' ? 'orange' : 'red' %>">Missing Details!</a>
<% } %>
<% if (allScans[i].double) { %>
<a href="<%- allScans[i].link %>" style="color: red">Duplicate UUID!</a>
<% if (item.duplicated) { %>
<a href="<%- item.link %>" style="color: red">Duplicate UUID!</a>
<% } %>
</li>
<% } %>
Expand Down