Allow Admins to transfer ownership of a project to a new address

divine-comedian commented 1 month ago

reported by @WhyldWanderer - we spend a notable amount of support and developer resources to handle every request by a project owner to transfer the ownership of their projects to a new address. The purpose of this issue is to create a stable and reliable way for admins to transfer the ownership of projects to a new address.

This feature would be accessible from the adminJS panel. Eventually we should be able to allow users to handle transferring project ownership themselves, but for now let's make it so at least admins can do it without needing a developer.

Requirements

[x] - Admin can see current project owner address while in project tab in AdminJS panel
[x] - Admin edits a project and enters a new owner address and a button to submit change to DB.
[x] - Owner must have profile on Giveth (to meet current requirements to create a project on Giveth)
[x] - Admin receives descriptive error if address is malformatted or not checksummed
[x] - Admin receives descriptive error if address entered has incomplete or nonexistant Giveth profile
[x] - Admin can see a field of previous owner saved to project's information which contains address of previous project owner if ownership has been transferred (we don't need a full history, just the previous person the project was transferred from)
[ ] - email is sent to previous project owner notifying them of ownership transfer
[ ] - email is sent to new project owner notifying them of ownership transfer

RamRamez commented 1 month ago

@ae2079 Could you please take this issue?

RamRamez commented 1 month ago

@ae2079 @divine-comedian JFYI, transferring the ownership right now is possible via AdminJS project tab select desired project -> click edit -> change Admin User Id to the new user id

Other items like sending emails and seeing list of previous owners still need to be implemented.

WhyldWanderer commented 1 month ago

@RamRamez - this issue was created because recently there have been some small bugs caused by using the AdminBro to transfer ownership of projects. I think it has to do with the tables in the db...

@carlos knows better than I about the technical information about the problems.

divine-comedian commented 1 month ago

For transparency:

https://github.com/Giveth/impact-graph/issues/1702#event-13684315268 https://github.com/Giveth/impact-graph/issues/1602

RamRamez commented 1 month ago

While I'm investigating how to transfer ownership, I asked @ae2079 to work on below item:

Admin can see a field of previous owner saved to project's information which contains address of previous project owner if ownership has been transferred (we don't need a full history, just the previous person the project was transferred from)
email is sent to previous project owner notifying them of ownership transfer
email is sent to new project owner notifying them of ownership transfer

@divine-comedian For sending emails, I believe we need two Ortto templates.

divine-comedian commented 1 month ago

@WhyldWanderer will draft the required emails by end of next week (Aug. 9)

ae2079 commented 1 month ago

@divine-comedian in this part:

Admin can see a field of previous owner saved to project's information which contains address of previous project owner if ownership has been transferred (we don't need a full history, just the previous person the project was transferred from)

Would you like the user's wallet address or is the userId enough?

RamRamez commented 1 month ago

To allow admins to transfer ownership of a project, I had to remove the email addresses column from projects tab in AdminJS, because it was making the stuff buggy, I added a button Export Emails to projects tab (you should select some projects first), I had a call with @WhyldWanderer, showed her the demo and talked about if new flow to fetch email addresses is ok or not.

Transfer ownership is now possible via AdminJS projects tab (select desired project -> click edit -> change Admin User Id to the new user id) but we don't have any of the requirements mentioned in this issue (actually AdminJS is not very customizable and I'm not sure if it's easy to implement all requirements).

I was working on the same code section so I asked @ae2079 not to start yet to prevent code conflicts. Now my work is done and @ae2079 can continue his work on this issue.

divine-comedian commented 1 month ago

@divine-comedian in this part:

Admin can see a field of previous owner saved to project's information which contains address of previous project owner if ownership has been transferred (we don't need a full history, just the previous person the project was transferred from)

Would you like the user's wallet address or is the userId enough?

The wallet address will actually be much more helpful in case we run into any user support issues.

@RamRamez @ae2079 - can you guys review the requirements and maybe explain what is easy to implement and what would be difficult? I don't feel we need to break our heads on this issue and I'm willing to eliminate some requirements.

ae2079 commented 1 month ago

@divine-comedian in this part:

Admin can see a field of previous owner saved to project's information which contains address of previous project owner if ownership has been transferred (we don't need a full history, just the previous person the project was transferred from)

Would you like the user's wallet address or is the userId enough?

The wallet address will actually be much more helpful in case we run into any user support issues.

@RamRamez @ae2079 - can you guys review the requirements and maybe explain what is easy to implement and what would be difficult? I don't feel we need to break our heads on this issue and I'm willing to eliminate some requirements.

actually, there is no difficulty in using the wallet address and we have both of them, but as we use userId in all relations we think maybe this one is more helpful to you.

For the second requirement, I'll check the adminJS features and if it has some limitations for that, tell them later.

ae2079 commented 1 month ago

@divine-comedian The second and third requirements seems impossible through AdminJS actions, so I made the owner address field editable in the project edit tab. With this one, we can change the owner of projects by address. (we can not do it for multiple projects at the same time and for each project, we should change the owner separately) Other requirements can be implemented and I'm currently working on them.

WhyldWanderer commented 1 month ago

Email draft can be found here...

I think its okay to use the same email for both cases. https://docs.google.com/document/d/16lVm7STIyc-yYk6KOHlr2q7NwISWMC40qymV3vVirvQ/edit?usp=sharing

divine-comedian commented 1 month ago

@divine-comedian The second and third requirements seems impossible through AdminJS actions, so I made the owner address field editable in the project edit tab. With this one, we can change the owner of projects by address. (we can not do it for multiple projects at the same time and for each project, we should change the owner separately) Other requirements can be implemented and I'm currently working on them.

Thanks Ali - I edited the requirements to reflect the limitations.

RamRamez commented 1 month ago

I think @ae2079 has completed about 90% of the requirements.

Only sending emails is remaining that we need an Ortto template for that, @ae2079 is that right? I know you are busy working on other projects right now, could you please check/select the completed items in the first comment of this issue?

ae2079 commented 1 month ago

I think @ae2079 has completed about 90% of the requirements.

Only sending emails is remaining that we need an Ortto template for that, @ae2079 is that right? I know you are busy working on other projects right now, could you please check/select the completed items in the first comment of this issue?

yes, just the Ortto schema remains and I sent you my proposal for this one. yes sure

ae2079 commented 1 month ago

I think @ae2079 has completed about 90% of the requirements. Only sending emails is remaining that we need an Ortto template for that, @ae2079 is that right? I know you are busy working on other projects right now, could you please check/select the completed items in the first comment of this issue?

yes, just the Ortto schema remains and I sent you my proposal for this one. yes sure

@RamRamez these are my assumed Ortto schema in coding:

{
  "activities": [
    {
      "activity_id": "act:cm:ownership-changed-to",
      "attributes": {
        "str:cm:ownername": "Ali",
        "str:cm:projectname": "Test Project",
      },
      "fields": {
        "str::email": "aliebrahimi2079@gmail.com",
        "str:cm::user-id": "1234"
      },
    }
  ],
  "merge_by": [
    "str::email",
    "str:cm::user-id",
  ]
}

{
  "activities": [
    {
      "activity_id": "act:cm:ownership-changed-from",
      "attributes": {
        "str:cm:ownername": "Ali",
        "str:cm:projectname": "Test Project",
      },
      "fields": {
        "str::email": "aliebrahimi2079@gmail.com",
        "str:cm::user-id": "1234"
      },
    }
  ],
  "merge_by": [
    "str::email",
    "str:cm::user-id",
  ]
}

divine-comedian commented 1 month ago

@ae2079

Sorry Ali, I think we only need to trigger one notification when the ownership transfer happens and you can replace the values sent accordingly.

Just checking if you have considered also if ownership is transferred to an address that has not yet created a profile - this should be possible and will probably be the most frequent request. In this case you would only need to trigger one notification - to the previous owner.

This is the schema you should use for the ortto activity trigger:

{
    "activities": [
        {
            "activity_id": "act:cm:ownership-transferred",
            "attributes": {
                "str:cm:email": "example string value",
                "str:cm:firstname": "example string value",
                "str:cm:newowneraddress": "example string value",
                "str:cm:projecttitle": "example string value",
                "str:cm:userid": "example string value"
            },
            "fields": {
                "str::email": "contact@email.com"
                "str:cm:user-id": "example string value"

            }
        }
    ],
}`;

ae2079 commented 1 month ago

@ae2079

Sorry Ali, I think we only need to trigger one notification when the ownership transfer happens and you can replace the values sent accordingly.

Just checking if you have considered also if ownership is transferred to an address that has not yet created a profile - this should be possible and will probably be the most frequent request. In this case you would only need to trigger one notification - to the previous owner.

This is the schema you should use for the ortto activity trigger:
{
  "activities": [
      {
          "activity_id": "act:cm:ownership-transferred",
          "attributes": {
              "str:cm:email": "example string value",
              "str:cm:firstname": "example string value",
              "str:cm:newowneraddress": "example string value",
              "str:cm:projecttitle": "example string value",
              "str:cm:userid": "example string value"
          },
          "fields": {
              "str::email": "contact@email.com"
              "str:cm:user-id": "example string value"

          }
      }
  ],
}`;

No problem, we can easily replace codes to use just one notification type. But about transferring ownership to an address that does not have a giveth profile, in the 5th requirement, you said we should check the address to have a complete profile, If you don't want this, we can remove this condition.

divine-comedian commented 1 month ago

Good point, so in order to respect this requirement let's make sure that when we check in the admin panel and transfer to an address that does NOT have a profile that we throw a descriptive error for the admin

"The entered address {address} does not have a profile on Giveth"

ae2079 commented 1 month ago

Good point, so in order to respect this requirement let's make sure that when we check in the admin panel and transfer to an address that does NOT have a profile that we throw a descriptive error for the admin

"The entered address {address} does not have a profile on Giveth"

yes, we show a red error text under the address input box

ae2079 commented 1 month ago

@ae2079

Sorry Ali, I think we only need to trigger one notification when the ownership transfer happens and you can replace the values sent accordingly.

Just checking if you have considered also if ownership is transferred to an address that has not yet created a profile - this should be possible and will probably be the most frequent request. In this case you would only need to trigger one notification - to the previous owner.

This is the schema you should use for the ortto activity trigger:
{
  "activities": [
      {
          "activity_id": "act:cm:ownership-transferred",
          "attributes": {
              "str:cm:email": "example string value",
              "str:cm:firstname": "example string value",
              "str:cm:newowneraddress": "example string value",
              "str:cm:projecttitle": "example string value",
              "str:cm:userid": "example string value"
          },
          "fields": {
              "str::email": "contact@email.com"
              "str:cm:user-id": "example string value"

          }
      }
  ],
}`;

@divine-comedian @RamRamez As I am completely on the Qacc project, Can we hold progress on this task until the end of Qacc or should I delegate this fix to another person?

divine-comedian commented 1 month ago

We can hold on this as long as @WhyldWanderer can confirm that there are no bugs currently for changing the owner via the admin panel

WhyldWanderer commented 1 month ago

Ao far, I have not encountered any bugs when changing adminuserid field.

Giveth / impact-graph

Allow Admins to transfer ownership of a project to a new address #1722

Requirements