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

202408 bankconfig #387

Closed
wants to merge 16 commits into from
Closed

202408 bankconfig #387

wants to merge 16 commits into from

Conversation

jbueren
Copy link
Contributor

@jbueren jbueren commented Oct 8, 2024

No description provided.

@jbueren jbueren requested a review from bblessmann October 8, 2024 10:26
Copy link
Member

@bblessmann bblessmann left a comment

Choose a reason for hiding this comment

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

ein paar Kleinigkeiten ;)


die "No valid integer for months" if $data->{months} && $data->{months} !~ /^[1-9][0-9]*$/;
die "No valid value for overwrite" if $data->{overwrite} && $data->{overwrite} !~ /^(0|1)$/;
die "No valid value for overwrite" if $data->{dry_run} && $data->{dry_run} !~ /^(0|1)$/;
Copy link
Member

Choose a reason for hiding this comment

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

falscher Text in Fehlermeldung

SL/BackgroundJob/SetBankAccountsMasterData.pm Show resolved Hide resolved
my (%params) = @_;

die "Need valid vc param, got" . $params{vc} unless $params{vc} && $params{vc} =~ /^(customer|vendor)$/;
die "Need valid months param, got" . $params{months} unless $params{months} && $params{months} =~ /^[1-9][0-9]*$/;
Copy link
Member

Choose a reason for hiding this comment

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

Hinter dem "got" würde ich einen Doppelpunkt oder ein Leerteichen spendieren, sonst ist die Fehlermeldung schlecht lesbar. Und evtl. sogar noch Anführungszeichen.

Wenn $params{vc} oder $params{months} undef ist, gibt es eine Warnung: "Use of uninitialized value $params{"vc"} in concatenation" oder so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

: undef;

die "Invalid state" unless $arap;

Copy link
Member

Choose a reason for hiding this comment

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

Das scheint mir kompliziert bzw. auch das "die" überflüssing. An dieser Stelle kann $params{vc} doch nur customer oder vendor sein:
my $arap = $params{vc} eq 'customer' ? 'ar' : 'ap';

Bei der vc_id prüfst Du ja auch nicht, ob params{vc} was anderes sein könnte.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

das die ist raus

AND bta.${arap}_id is not NULL
GROUP BY bt.remote_account_number,bt.remote_bank_code, $vc_id
ORDER BY $vc_id
SQL
Copy link
Member

Choose a reason for hiding this comment

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

Ich würde die Schlüsselwörter durchgängig groß schreiben (select/distinct/ on/in/not null/from/where/...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

[% PROCESS 'client_config/_orders_deleteable.html' %]
[%- IF INSTANCE_CONF.get_doc_storage %]
[% PROCESS 'client_config/_attachments.html' %]
[%- END %]
Copy link
Member

Choose a reason for hiding this comment

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

Warum nicht mehr einrücken?
Und Kosmetik gerne extra ;)

locale/de/all Outdated
'If enabled the invoice number will be copied to the unstructured message upon booking. If there\'s a message already the invoice number will be appended to it.' => 'Falls aktiviert, wird die Rechnungsnummer beim Buchen automatisch in die unstrukturierte Mitteilung kopiert. Falls diese bereits eine Mitteilung enthält, wird die Rechnungsnummer an diese angehängt.',
'If enabled the record links view starts always from the sales order including all sublevels' => 'Falls aktiv, werden die verknüpften Belege immer vom Verkaufsauftrag inkl. aller darunterliegenden Belege angezeigt',
'If enabled the tab Proposals will not be the default view for bank transaction bookings' => 'Falls aktiviert springt die Ansicht beim Kontoauszug verbuchen nicht direkt auf den Reiter Vorschläge sind zeigt immer Alle Buchungen an.',
Copy link
Member

Choose a reason for hiding this comment

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

typo: "sondern" statt "sind"

locale/de/all Outdated
@@ -3382,6 +3385,8 @@ $ ./scripts/installation_check.pl',
'SEPA message ID' => 'SEPA-Nachrichten-ID',
'SEPA message IDs' => 'SEPA-Nachrichten-IDs',
'SEPA strings' => 'SEPA-Überweisungen',
'SEPA-XML File ending' => 'SEPA-XML Dateiendung',
Copy link
Member

Choose a reason for hiding this comment

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

"extension" statt "ending"?

<tr>
<th>[% LxERP.t8('SEPA-XML File ending') %]</th>
<td>[% L.yes_no_tag('defaults.sepa_export_xml', SELF.defaults.sepa_export_xml) %]</td>
<td class="longdesc">[% LxERP.t8('If enabled the file ending for SEPA exports will be .xml instead of .cct or .cdd') %]</td>
Copy link
Member

Choose a reason for hiding this comment

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

"extension" statt "ending"?

locale/de/all Outdated
@@ -4438,6 +4443,7 @@ $ ./scripts/installation_check.pl',
'This invoice was added from an order. See there.' => 'Diese Rechnung wurde aus einem Auftrag erstellt. Siehe dort.',
'This invoice\'s dunning level: #1' => 'Mahnstufe dieser Rechnung: #1',
'This is a very critical problem.' => 'Dieses Problem ist sehr schwerwiegend.',
'This is helpful if you prefer a chronologic order of bank transactions, i.e. the tab all transactions meanwhile you are still able to use the quick add link for the suggested proposals' => 'Das ist nützlich falls man eine rein chronologische Reihenfolge beim Kontoauszug verbuchen möchte, die Vorschläge werden ja dennoch generiert und sind über den anklickbaren Link auch direkt verfügbar.',
Copy link
Member

Choose a reason for hiding this comment

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

Komma vor "falls"

@bblessmann
Copy link
Member

Ah - sorry. Habe "approved" geklickt. Ein paar Dinge sind aber noch zu tun ;)

Copy link
Member

@bblessmann bblessmann left a comment

Choose a reason for hiding this comment

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

Kleinigkeiten sind noch zu tun ;)

@jbueren jbueren closed this Jan 4, 2025
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.

3 participants