TwingleShop: add setting for the financial type of additional donations#122
TwingleShop: add setting for the financial type of additional donations#122MarcMichalsky wants to merge 2 commits intosystopia:masterfrom
Conversation
|
Yes I can test the PR. Do you plan to fix the failed checks before manual testing? |
|
I will not test the installation of the extension to a fresh CiviCRM installation. In a previous issue we had the problem, that some extra code was missing for the initial installation, to make the corresponding database schema changes. Is this covered by the pull request? |
|
Hi @GFF-DS,
No, I don't plan to fix the failed tests. The code sniffer actually complains about the syntax throughout the entire file. If I were to fix all of that, this PR would no longer be anywhere near atomic. The PHPStan test fails already in the setup of the test environment.
No, it's not covered in this PR but was covered in this already merged PR: #114 |
ok, thanks for the explanation!
ah, sorry, I mixed up that for new settings there is no need to adapt the database schema. I will test the PR probably next week. |
| elseif ($product['id'] === -22) { | ||
| // Identify additional donations by their ID. | ||
| // See https://github.com/systopia/de.systopia.twingle/issues/102 | ||
| elseif ($product['id'] === self::TWINGLE_ADDITIONAL_DONATION_ID) { |
There was a problem hiding this comment.
Perfect! Just nitpicking: this could preferably be a yoda condition.
There was a problem hiding this comment.
Hurt my brain, the Yoda condition coding style does.
|
We should squash this PR before merging. |
|
Currently, there is a msgid "Financial Type for top up donations", which currently is translated into german as "Zuwendungsart für zusätzliche Spenden". The PR adds a msgid "Financial Type for additional donations", which I think is not translated in the PR (?). My suggestion would be to
I will continue testing and come back with a feedback about the functionality itself. |
|
In terms of functionality, the PR works. When in the twingle profile I choose "Produkte Preisfeldern zuordnen", save the profile and edit it again, the price fields are not loaded. However, I never used this feature, so that I don't know if I maybe don't use it correctly. For us, the general settings about the financial_type are sufficient. |
Yes, good point. I'll change that translation and add the new one. Thanks! |
- Adds a translation string for "Financial Type for additional donations". - Improves German translations for additional and top up donations. See: systopia#122 (comment)
Allows specifying a financial type for additional donations. Uses a constant for the fixed ID associated with additional donations and improves German translations for clarity.
MarcMichalsky
force-pushed
the
option_for_additional_donations
branch
from
c91f744 to
184114d
Compare
|
I have updated the translations and squashed the commits. |
MarcMichalsky
left a comment
MarcMichalsky
left a comment
There was a problem hiding this comment.
Ready to merge!
| "Mitgliedschaftsobjekt erstellt, können Sie einen API-Aufruf (als " | ||
| "'Entität.Aktion') angeben, um neu erstellt Mitgliedschaften an die " | ||
| "Anforderungen Ihrer Organisation anzupassen." | ||
|
|
There was a problem hiding this comment.
Don't be confused. Poedit seems to have changed the line splitting a little. I could have discarded these changes, but they are probably included in the compiled .mo file, so I decided against it.
The issue was introduced in CiviCRM version 6.11.0
3 participants
I added a setting field for the financial type of additional donations.
As we learned in #102, the
idof additional donations is always-22. The default financial type is1, which should be "donation" in most cases.Could you please test the PR @GFF-DS?
fix #102