DCI number conversion and validation #29

ChaimW · 2017-07-02T07:16:16Z

Completes Issue #4

ChaimW · 2017-07-05T22:25:01Z

@Nightfirecat @april

Nightfirecat · 2017-07-05T22:29:06Z

I'll try to load this up and test after work today.

ChaimW · 2017-07-05T22:30:37Z

i have it running at http://csun.edu/~csw61793/decklist

Nightfirecat

These are mostly stylistic changes, will verify algorithm and logic shortly, but the demo seems to be working fine.

An aside, I feel we may want to add a note if the DCI number is rewritten. Not a warning, necessarily, but just a notice so the user is aware that 10-digit DCI numbers are desired, and what their 10-digit DCI number would be.

Nightfirecat · 2017-07-06T03:25:54Z

js/decklist/dci.js

@@ -0,0 +1,71 @@
+var primes = [43,47,53,71,73,31,37,41,59,61,67,29];
+function isValid(number){
+  number += "";


number = number.toString();

Nightfirecat · 2017-07-06T03:28:05Z

js/decklist/dci.js

+function isValid(number){
+  number += "";
+  if(number.length >= 8){
+    if(number.length == 8 || number.length == 10){


if (, not if(
) {, not ){

Nightfirecat · 2017-07-06T03:28:37Z

js/decklist/dci.js

+var primes = [43,47,53,71,73,31,37,41,59,61,67,29];
+function isValid(number){
+  number += "";
+  if(number.length >= 8){


if (, not if(
) {, not ){

Nightfirecat · 2017-07-06T03:29:39Z

js/decklist/dci.js

@@ -0,0 +1,71 @@
+var primes = [43,47,53,71,73,31,37,41,59,61,67,29];
+function isValid(number){


Maybe these functions should be more descriptively named? Or perhaps nested so that they don't bleed into global namespace?

e: Looking at this comment again, there may not be as much room to do this as I thought; just was a thought of mine.

) {, not ){

Functions nested, spacing changed

Nightfirecat · 2017-07-06T03:30:12Z

js/decklist/dci.js

+  else return -1; // for a under 8-digit number, return -1 (evals to true)
+}
+function getTenDigit(number){
+  number += "";


number = number.toString();

Nightfirecat · 2017-07-06T03:42:26Z

js/decklist/dci.js

+	for (var i = 0; i < number.length; i++){
+		sum += parseInt(number[i]) * primes[i];
+	}
+	var check = (Math.floor(sum / 10) % 9) + 1


Missing semicolon

Nightfirecat · 2017-07-06T03:42:36Z

js/decklist/dci.js

+	var check = (Math.floor(sum / 10) % 9) + 1
+	return check;
+}
+function getTenIfValid(number){


) {, not ){

Nightfirecat · 2017-07-06T03:42:49Z

js/decklist/dci.js

+	return check;
+}
+function getTenIfValid(number){
+  if(getTenDigit(number)){


if (, not if(
) {, not ){

Nightfirecat · 2017-07-06T03:43:09Z

js/decklist/main.js

@@ -327,6 +327,10 @@ function generateDecklistPDF(outputtype) {
  }

  dcinumber = $('#dcinumber').val();
+  if(dcinumber){ // only if there is a dci number


if (, not if(
) {, not ){

Nightfirecat · 2017-07-06T03:43:52Z

js/decklist/main.js

@@ -327,6 +327,10 @@ function generateDecklistPDF(outputtype) {
  }

  dcinumber = $('#dcinumber').val();
+  if(dcinumber){ // only if there is a dci number
+    dcinumber = getTenIfValid(dcinumber);
+    dcinumber += ""; //convert to string (function returns an int)


dcinumber = getTenIfValid(dcinumber).toString();

Nightfirecat · 2017-07-06T03:49:41Z

BTW, you can also name your branch gh-pages (or configure the rendered page in your repo settings) rather than manually hosting it elsewhere. Just so you know. ;)

ChaimW · 2017-07-06T06:30:26Z

Yeah, i was trying to test some things while my dev machine was acting up, so all I had was my phone. It was fairly simple to ssh to my school stuff and do a clone, rather than teach myself github pages.

ChaimW · 2017-07-07T08:19:39Z

@Nightfirecat I think I got them all. I also updated the host.

ChaimW · 2017-07-10T19:12:21Z

@april Is there something further that I need to do for this? this is my first real pull request, so I'm slightly unfamiliar with how to do it.

april · 2017-07-10T19:14:39Z

Sorry, I've been traveling the last couple weeks. Could I ask you to squash it all into a single commit, please?

april · 2017-07-10T19:15:24Z

js/decklist/main.js

+        } else if (validationObject['error'] === 'invalid') {
+          notifications.push(prop, ['DCI number is invalid', validType]);
+        } else if (validationObject['warning'] === 'nocheck') {
+          notifications.push(prop, ['We cannot verify that your DCI# is valid as it is in an old format. Please double-check it.', validType]);


Please be consistent about using DCI Number vs. DCI #.

oops, my bad.

ChaimW · 2017-07-10T23:16:27Z

@april Squashed, updating host.

Nightfirecat · 2017-07-11T01:39:44Z

Looks good as far as I'm concerned. Style changes are mostly consistent with existing code, and the algorithms seem sound.

ChaimW · 2017-07-31T08:57:34Z

Hey, @april I just wanted to know if this can get pulled so that I can work with a current copy when I start on #11. I plan to start that once this is closed.

april · 2017-07-31T21:36:28Z

If you're on Twitter, I'd be happy to credit you. :)

ChaimW · 2017-08-01T15:05:40Z

I'm on Twitter @imanEngineer2

tooomm · 2017-12-05T13:40:39Z

@Nightfirecat wrote:
BTW, you can also name your branch gh-pages (or configure the rendered page in your repo settings) rather than manually hosting it elsewhere. Just so you know. ;)

Is that still true? That would be damn sweet!!

@april Do you run decklist.org directly from gh pages?

Nightfirecat · 2017-12-05T17:25:00Z

I don't believe so, since GH pages (still?) doesn't allow for custom certs on custom domains. That said, you can definitely host it from GH pages in development: https://nightfirecat.github.io/decklist/

april · 2017-12-05T17:26:00Z

I can probably push it to gh-pages, if folks would like. I can't run it directly from GitHub because as @Nightfirecat said, they don't yet support certificates on custom domains.

tooomm · 2017-12-05T21:27:53Z

Which certificates are we talking about? What are they used for?

you can definitely host it from GH pages in development

GH pages in development? Hmm, what do you mean?

Nightfirecat · 2017-12-05T21:42:23Z

Server HTTPS cert. And by "in development", I mean you can host directly from GitHub on *.github.io from a repository's development code.

tooomm · 2017-12-05T22:26:04Z

Oh right, it could be production code too then. ;)

HTTPS is actually possible with gh pages and a custom domain if you combine it with a free cloudflare account. Just do a quick google search, there are a lot of articles. If you want to safe some hosting costs...

Nightfirecat · 2018-04-03T18:30:44Z

@april FYI, there's a fair bit of discussion on isaacs/github#156 regarding a slow roll-out of custom domain certs on GitHub's part for GH pages. It's not immediately available, but at some point, it sounds like this will be fully supported and available for new sites. Once that's available, it might be worth just running decklist.org off GH pages?

ChaimW mentioned this pull request Jul 5, 2017

Old DCI Number Conversion #4

Open

Nightfirecat reviewed Jul 6, 2017

View reviewed changes

april reviewed Jul 10, 2017

View reviewed changes

Add DCI number validation and expansion

a6ed821

april merged commit 85f1b49 into april:master Jul 31, 2017

ChaimW deleted the dci-numbers branch August 1, 2017 15:04

Nightfirecat mentioned this pull request Apr 19, 2018

enable/run from gh pages #44

Closed

		@@ -0,0 +1,71 @@
		var primes = [43,47,53,71,73,31,37,41,59,61,67,29];
		function isValid(number){

DCI number conversion and validation #29

DCI number conversion and validation #29

Conversation

ChaimW commented Jul 2, 2017

ChaimW commented Jul 5, 2017

Nightfirecat commented Jul 5, 2017

ChaimW commented Jul 5, 2017

Nightfirecat left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nightfirecat Jul 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nightfirecat commented Jul 6, 2017

ChaimW commented Jul 6, 2017 • edited Loading

ChaimW commented Jul 7, 2017

ChaimW commented Jul 10, 2017

april commented Jul 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChaimW commented Jul 10, 2017

Nightfirecat commented Jul 11, 2017

ChaimW commented Jul 31, 2017

april commented Jul 31, 2017

ChaimW commented Aug 1, 2017

tooomm commented Dec 5, 2017

Nightfirecat commented Dec 5, 2017

april commented Dec 5, 2017

tooomm commented Dec 5, 2017

Nightfirecat commented Dec 5, 2017

tooomm commented Dec 5, 2017

Nightfirecat commented Apr 3, 2018

Nightfirecat left a comment •

edited

Loading

Nightfirecat Jul 6, 2017 •

edited

Loading

ChaimW commented Jul 6, 2017 •

edited

Loading