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

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

Open
6 of 8 tasks
divine-comedian opened this issue Jul 24, 2024 · 23 comments
Open
6 of 8 tasks
Assignees
Labels

Comments

@divine-comedian
Copy link
Collaborator

divine-comedian commented Jul 24, 2024

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

  • - Admin can see current project owner address while in project tab in AdminJS panel
  • - Admin edits a project and enters a new owner address and a button to submit change to DB.
  • - Owner must have profile on Giveth (to meet current requirements to create a project on Giveth)
  • - Admin receives descriptive error if address is malformatted or not checksummed
  • - Admin receives descriptive error if address entered has incomplete or nonexistant Giveth profile
  • - 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
@github-project-automation github-project-automation bot moved this to New Issues in All-Devs Jul 24, 2024
@divine-comedian divine-comedian moved this from New Issues to Product Backlog in All-Devs Jul 24, 2024
@RamRamez
Copy link
Collaborator

@ae2079 Could you please take this issue?

@RamRamez RamRamez assigned ae2079 and unassigned CarlosQ96 Jul 29, 2024
@RamRamez
Copy link
Collaborator

@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
Copy link

@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
Copy link
Collaborator Author

divine-comedian commented Jul 30, 2024

For transparency:

image
#1702 (comment)
#1602

@RamRamez
Copy link
Collaborator

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
Copy link
Collaborator Author

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

@ae2079
Copy link
Contributor

ae2079 commented Aug 1, 2024

@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
Copy link
Collaborator

RamRamez commented Aug 1, 2024

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
Copy link
Collaborator Author

@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
Copy link
Contributor

ae2079 commented Aug 1, 2024

@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
Copy link
Contributor

ae2079 commented Aug 5, 2024

@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
Copy link

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
Copy link
Collaborator Author

@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
Copy link
Collaborator

RamRamez commented Aug 9, 2024

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
Copy link
Contributor

ae2079 commented Aug 11, 2024

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
Copy link
Contributor

ae2079 commented Aug 11, 2024

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": "[email protected]",
        "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": "[email protected]",
        "str:cm::user-id": "1234"
      },
    }
  ],
  "merge_by": [
    "str::email",
    "str:cm::user-id",
  ]
}

@divine-comedian
Copy link
Collaborator Author

divine-comedian commented Aug 12, 2024

@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": "[email protected]"
				"str:cm:user-id": "example string value"

			}
		}
	],
}`;

@ae2079
Copy link
Contributor

ae2079 commented Aug 12, 2024

@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": "[email protected]"
				"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
Copy link
Collaborator Author

divine-comedian commented Aug 13, 2024

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
Copy link
Contributor

ae2079 commented Aug 13, 2024

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
Copy link
Contributor

ae2079 commented Aug 13, 2024

@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": "[email protected]"
				"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
Copy link
Collaborator Author

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

@divine-comedian divine-comedian moved this from In Progress to Product Backlog in All-Devs Aug 14, 2024
@WhyldWanderer
Copy link

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

@divine-comedian divine-comedian moved this from Product Backlog to Icebox in All-Devs Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Icebox
Development

No branches or pull requests

5 participants