Re: task 8833 -- proposed steps

[Davi Leal]

Federico Gimenez Nieto wrote:
> Ok, following your indications i think that this would be the steps in
> order to send confirmation email to registered but not logged users
> which want to add donations to a donation-pledge-group:


> 1.- Modify the database schema, adding the fields :
>
>     R1_DonationMagic (string, default null)
>     R1_DonationMagicExpire (date, default null)

IMHO, it should be:
      R1_DonationMagic        varchar(512) DEFAULT NULL,
      R1_DonationMagicExpire  timestamp NOT NULL DEFAULT 'now',

Similar to what we use at:
      E1_WantEmailMagic       varchar(512) DEFAULT NULL
      E1_WantEmailMagicExpire timestamp NOT NULL DEFAULT 'now',

> to the R1_Donations2JobOffersJoins table (could it be better to add a
> separate R1_Donations table?)

Hmm. Maybe we could just rename the R1_Donations2JobOffersJoins table to 
D1_Donations.  We could think a week more about such renaming. If we follow 
thinking that the next Saturday then we can do such renaming.

Proposal:  "Development rules"

  When we are not 100% sure, letting us a week more to think we avoid
  adding non-so-much-good patches to production.


> 2.- Modify the current database, adding this modifications


> 3.- Modify Layer-5__DB_operation/Job_offer.php, addDonation method
>     to allow inserting registers with the values of R1_DonationMagic
>     and R1_DonationMagicExpire to R1_Donations2JobOffersJoins table
>     (perhaps this could be carried by a separate class, if we would
>     had a separate R1_Donations table)

Interesting, about having a Donations class. Methods to move to such 
Layer-5__DB_operation/Donation.php class:

  * getDonators($JobOfferId)
  * getDonationsForPledgeGroup($JobOfferId)
  * addDonation($JobOfferId)
  * cancelSelectedDonations()
  * getMyDonations()
  * IsAlreadyDonator($EntityId,$JobOfferId)

Let us wait a week more, and if all is right we could write a patch for such 
table-renaming and class-addition the next Saturday.


> 4.- Modify getEntityId method of Layer-5__DB_operation/Entity.php:
>
>     public function getEntityId($email, $requestOperation){
>     ...
>     if ( $E1_Id )
>     {
>       if($requestOperation=='REQUEST_TO_ADD_DONATION_TO_NOTICE')
>         {
>           // create magic number and insert register at
>           // R1_Donations2JobOffersJoins with the values for
>           // R1_DonationMagic and R1_DonationMagicExpire
>
>           // create the body of the message (including verification
>           // link) and send
>
>         }
>       return $E1_Id;
>     }


> 5.- For verifying the sent mail, add entry point and verification method to
>     Layer-2__Business_logic/content/forms/Donation_Pledge_Group_form.php 

The entity-email verification link is:
  https://gnuherds.org/entity?action=register&email=&magic=

Verification link for donations could be: I am not sure.
  https://gnuherds.org/pledges?action=donate&email=&magic=


> 6.- Modify the donations getters to show only verified donations,
>     at Layer-5__DB_operation/Job_Offer.php in this methods:
>
>       * getDonators($JobOfferId)
>       * getDonationsForPledgeGroup($JobOfferId)
>       * getMyDonations()

Proposal:

  Shows too non-verified donations. So the webapp will give immediate
  feedback to who add a donation, even if [s]he is not logged.

  If it is not verified it will just deleted. So no problem.

IMHO, the webapp giving feedback to users is good. When I donate I want to see 
my donation listed immediately, even if I will verify tomorrow.


> 7.- Add the cronjob to delete the R1_Donations2JobOffersJoins
>     expired registers.



> What do you think?

Maybe we will have additional comment after the patch is done.


> Just as a thought, the getEntityId method of
>   Layer-5__DB_operation/Entity.php
> cares of matters not only related with db operations, such as
> creating and sending emails.  Shouldn't this be carried out by
> classes at a not so deep layer, for example at Layer-2?

Note right now the getEntityId method is used for two JobOffer methods:

 * getEntityId(trim($_POST['Email']),'REQUEST_TO_ADD_NOTICE');
 * getEntityId(trim($_POST['Email']),'REQUEST_TO_ADD_DONATION_TO_NOTICE');

If we move out the code which send the email we will have duplicate it. And 
duplication is not good for maintenance neither for code readability.

Additionally, note if we keep it inside such method we could use a common text 
for all email. A more abstract and common text will do the translator task 
easier due to they will have less msgid to translate.


Proposed "policy":

  Try to reduce the number, length and complexity of msgid to translate if
  it is possible, but without damage the webapp. Just use the common sense.



Anyway, as usual, I could be mistaken.

What do you think?