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

Add actual fail count to Result buttons #96

Closed
KlaasBonnema opened this issue Aug 26, 2023 · 9 comments
Closed

Add actual fail count to Result buttons #96

KlaasBonnema opened this issue Aug 26, 2023 · 9 comments

Comments

@KlaasBonnema
Copy link

The results column on the Report list shows a DKIM and SPF button with a red or green text. A red text does not give any indication about how many failures are actually reported. If the Messages column has a count of 100 then there could be anywhere between 1 or 100 failures.
Suggest to report an actual DKIM failure count of 5 like "DKIM: 5". This way you know how urgently you need to dig deeper.

@williamdes
Copy link
Contributor

I agree, I have some people that receive the emails that must be doing forwarding and always report broken dkim and spf
So basically all reports are red because one entry in the report is always failing out of my control

@KlaasBonnema
Copy link
Author

KlaasBonnema commented Aug 27, 2023

I made the following changes to make this happening:
ReportMapper.php

    public function list(array &$filter, array &$order, array &$limit): array
    {
        $db = $this->connector->dbh();
        $list = [];
        $f_data = $this->prepareFilterData($filter);
        $order_str = $this->sqlOrderList($order, '`rp`.`id`');
        $cond_str0 = $this->sqlConditionList($f_data, ' AND ', 0);
        $cond_str1 = $this->sqlConditionList($f_data, ' HAVING ', 1);
        $limit_str = $this->sqlLimit($limit);
        try
        {
            $st = $db->prepare('SELECT `org`, `begin_time`, `end_time`, `fqdn`, `external_id`, `seen`, SUM(`rcount`) AS `rcount`,' . ' MIN(`dkim_align`) AS `dkim_align`, MIN(`spf_align`) AS `spf_align`,' . ' MIN(`disposition`) AS `disposition` 
, (SELECT IFNULL(SUM(f2.rcount),"") FROM `' . $this->connector->tablePrefix('rptrecords') . '` AS `f2` WHERE f2.report_id=rr.report_id AND f2.dkim_align<2) AS dkim_fail_count
, (SELECT IFNULL(SUM(f3.rcount),"") FROM `' . $this->connector->tablePrefix('rptrecords') . '` AS `f3` WHERE f3.report_id=rr.report_id AND f3.spf_align<2) AS spf_fail_count
FROM `' . $this->connector->tablePrefix('rptrecords') . '` AS `rr` 
RIGHT JOIN (SELECT `rp`.`id`, `org`, `begin_time`, `end_time`, `external_id`,' . ' `fqdn`, `seen` FROM `' . $this->connector->tablePrefix('reports') . '` AS `rp` 
INNER JOIN `' . $this->connector->tablePrefix('domains') . '` AS `d` ON `d`.`id` = `rp`.`domain_id`' . $cond_str0 . $order_str . ') AS `rp` ON `rp`.`id` = `rr`.`report_id` GROUP BY `rp`.`id`' . $cond_str1 . $order_str . $limit_str);
            $this->sqlBindValues($st, $f_data, $limit);
            $st->execute();
            while ($row = $st->fetch(\PDO::FETCH_NUM))
            {
                $list[] = [
                    'org_name' => $row[0],
                    'date' => [
                        'begin' => new DateTime($row[1]),
                        'end' => new DateTime($row[2])
                    ],
                    'domain' => $row[3],
                    'report_id' => $row[4],
                    'seen' => (bool) $row[5],
                    'messages' => $row[6],
                    'dkim_align' => Common::$align_res[$row[7]],
                    'spf_align' => Common::$align_res[$row[8]],
                    'disposition' => Common::$disposition[$row[9]],
                    'dkim_fail_count' => $row[10],
                    'spf_fail_count' => $row[11]
                ];
            }
            $st->closeCursor();
        } catch (\PDOException $e)
        {
            throw new DatabaseFatalException('Failed to get the report list', - 1, $e);
        }
        return $list;
    }

list.js:

	_make_row_data(d) {
		let rd = {
			cells: [],
			userdata: { domain: d.domain, time: d.date.begin, org: d.org_name, report_id: d.report_id },
			seen: d.seen && true || false
		};
		rd.cells.push({ content: d.domain, label: "Domain" });
		let d1 = new Date(d.date.begin);
		let d2 = new Date(d.date.end);
		rd.cells.push({ content: date_range_to_string(d1, d2), title: d1.toUIString(true) + " - " + d2.toUIString(true), label: "Date" });
		rd.cells.push({ content: d.org_name, label: "Reporting Organization" });
		rd.cells.push({ content: d.report_id, class: "report-id" });
		rd.cells.push({ content: d.messages, label: "Messages" });
		rd.cells.push(new StatusColumn({ dkim_align: d.dkim_align, spf_align: d.spf_align,dkim_fail_count: d.dkim_fail_count, spf_fail_count:d.spf_fail_count }));
		return rd;
	}
class StatusColumn extends ITableCell {
	element() {
		if (!this._element) {
			super.element().setAttribute("data-label", "Result");
		}
		return this._element;
	}

	value(target) {
		if (target === "dom") {
			let d = this._content;
			let fr = document.createDocumentFragment();
			if (d.dkim_align) {
				fr.appendChild(create_report_result_element("DKIM", d.dkim_fail_count,d.dkim_fail_count,d.dkim_align));
			}
			if (d.spf_align) {
				fr.appendChild(create_report_result_element("SPF", d.spf_fail_count,d.spf_fail_count,d.spf_align));
			}
			return fr;
		}
		return super.value(target);
	}
}

@williamdes
Copy link
Contributor

Can you edit your post and use fences?


```php
// code
```

@KlaasBonnema
Copy link
Author

KlaasBonnema commented Aug 27, 2023

I don't understand what you mean with fences.

Following is the actual lines that were changed. I copied complete functions before:

$st = $db->prepare('SELECT org, begin_time, end_time, fqdn, external_id, seen, SUM(rcount) AS rcount,' . ' MIN(dkim_align) AS dkim_align, MIN(spf_align) AS spf_align,' . ' MIN(disposition) AS disposition
, (SELECT IFNULL(SUM(f2.rcount),"") FROM ' . $this->connector->tablePrefix('rptrecords') . ' AS f2 WHERE f2.report_id=rr.report_id AND f2.dkim_align<2) AS dkim_fail_count
, (SELECT IFNULL(SUM(f3.rcount),"") FROM ' . $this->connector->tablePrefix('rptrecords') . ' AS f3 WHERE f3.report_id=rr.report_id AND f3.spf_align<2) AS spf_fail_count
'dkim_fail_count' => $row[10],
'spf_fail_count' => $row[11]

fr.appendChild(create_report_result_element("DKIM", d.dkim_fail_count,d.dkim_fail_count,d.dkim_align));

fr.appendChild(create_report_result_element("SPF", d.spf_fail_count,d.spf_fail_count,d.spf_align));

@williamdes
Copy link
Contributor

I don't understand what you mean with fences.

You can edit your comments and use my example

```php
$st = $db->prepare('SELECT org, begin_time, end_time, fqdn, external_id, seen, S...
```

will look like

$st = $db->prepare('SELECT org, begin_time, end_time, fqdn, external_id, seen, S...

@KlaasBonnema
Copy link
Author

@williamdes, Thanks - I learned something new :-)

@williamdes
Copy link
Contributor

Thank you!
I looks 200% better
It works on many Markdown based platforms, like Stackoverflow

You have one last message to fence just before my example :)

Also, you can propose diffs using diff

Like

- $foo = 1;
+ $foo = 2;

@liuch
Copy link
Owner

liuch commented Aug 28, 2023

I think this idea is worth implementing. But I need to think about a more optimal SQL query, as nested queries don't seem to be the optimal solution to me.

@liuch
Copy link
Owner

liuch commented Jul 21, 2024

Implemented.

@liuch liuch closed this as completed Jul 21, 2024
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

No branches or pull requests

3 participants