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

Add the slack platform support #71

Merged
merged 23 commits into from
Feb 28, 2023
Merged

Add the slack platform support #71

merged 23 commits into from
Feb 28, 2023

Conversation

ash0521
Copy link
Contributor

@ash0521 ash0521 commented Feb 11, 2023

I hope this time will work,,,RUN

@BinaryHB0916
Copy link
Member

BinaryHB0916 commented Feb 12, 2023

Thanks, Ash. The community will check this by the following Monday.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 13, 2023

Could you please build now?

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 13, 2023

😪

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 14, 2023

Please take a closer look and say something

@nykma
Copy link
Member

nykma commented Feb 14, 2023

I'll take a close look after CI is passed.
I suggest you to build and test it locally before committing

  • Run gofmt -l . until no output
  • Run make build until no error
  • Write test of this validator (like other validators do in this project)

If you have no clue what I'm saying, learn how to programming using go first. This project is not udemy for you.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 14, 2023

Sorry,I forget gofmt, please stay online,I change now

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 14, 2023

Clear

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 14, 2023

Don't run,I will all checked then text you,when totally resolved

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 15, 2023

Man it has been a long time, Please finish

@ash0521 ash0521 closed this Feb 15, 2023
@ash0521 ash0521 reopened this Feb 15, 2023
@nykma
Copy link
Member

nykma commented Feb 15, 2023

Good, but we still need a test to represent the whole workflow (at least validation workflow). Especially, I want to know the format of field Identity and ProofLocation (and the reason). They will be all self-explained in unit tests so no need to reply to it here.

Also, don't put Slack API key in the test code, just code and run locally on your machine. Commit the core part when you're done.

Take validator/twitter/twitter_test.go or validator/minds/minds_test.go as samples.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 15, 2023

Build

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 16, 2023

sorry, i'm working.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 16, 2023

Please finish quickly

@nykma
Copy link
Member

nykma commented Feb 21, 2023

I follow telegram test code.

platforms differ widely, don't just copy / paste other platform's impl / test code. You can take them as a reference, but duplicate them must not work.

So you still don't understand the whole workflow of how ProofService works. Learn it in doc, and try to use it (MaskBook browser plugin's identity binding function) first. Until then, let's discuss about impl details.

@NextDotID NextDotID deleted a comment from ash0521 Feb 21, 2023
@ash0521
Copy link
Contributor Author

ash0521 commented Feb 21, 2023

@nykma
Copy link
Member

nykma commented Feb 21, 2023

Also, don't post private chat into this public discussion.

@nykma
Copy link
Member

nykma commented Feb 21, 2023

Check this,https://myworkspace.slack.com/archives/C1234567890/p123456789012345.

C1234567890? Is this real? Did you copy it from your workspace?

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 21, 2023

No,This is example.

@nykma
Copy link
Member

nykma commented Feb 21, 2023

What I need is a real ProofLocation in the test. You should run this test locally on your computer, and submit it when it's OK.

It will be better if you add some instruction of "how to get ProofLocation" in README.md.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 21, 2023

My public channel id,C04Q3P6H7TK

@nykma
Copy link
Member

nykma commented Feb 21, 2023

My public channel id,C04Q3P6H7TK

Write it in test then.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 21, 2023

And you say do we need to add slack,Slack is better than discord,Big company uses slack,Most slack users are company employees.

@nykma
Copy link
Member

nykma commented Feb 21, 2023

And you say do we need to add slack,Slack is better than discord,Big company uses slack,Most slack users are company employees.

I thought a little bit after that, I'm now realized that it worth to be supported, for on-premise scenario.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 21, 2023

Should we add page limit to our code?

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 23, 2023

I got ProofLocation shape, ProofLocation will be proof message link.

validator/slack/slack.go Outdated Show resolved Hide resolved
@ash0521
Copy link
Contributor Author

ash0521 commented Feb 24, 2023

Okay, Please help me in testing code
Check the minds testing reference
Can you please explain me the requirements of the testing code 🙏🙏
Can I get proof payload before code merge?
Also I can't generate signature in go
But ProofLocation and AltID is confirmed, ProofLocation will be proof message link and AltID will be UserID.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 24, 2023

Please tell🙏🙏

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 24, 2023

Please would you cooperate to end it?

@nykma
Copy link
Member

nykma commented Feb 24, 2023

Don't flood message in PR. Github is not IM, you should NOT expect immediate response from others.

Can you please explain me the requirements of the testing code

  1. Generate a keypair by your own, note down pubkey (you will use it in tests like here )
  2. Generate a sign payload (using your own GenerateSignPayload()) and a post payload (using your own GeneratePostPayload()) in your own playground mini program described below. Note down all necessary info to help you rebuild validator.Base in test case like this.
  3. Sign the sign payload using private key generated in 1 , and place Base64 sig into post payload generated in 2
  4. Publish / send / post the post payload in real world (in your case, send it in a slack channel), like the common user will use ProofService finally. Copy the proof location (in your case, maybe the message share link. If message link doesn't have the info you need to Validate(), you need to find another way), and use it in your test like this.
  5. Setup your slack API key locally, and run your test. If test went green, submit your code, not your API key.

Can I get proof payload before code merge?

Write a small cmd/playground-ish by yourself, and generate one using your own GeneratePostPayload() func. Should be not that difficult.
BTW, Don't submit your playground, write and use it locally for your own.

Also I can't generate signature in go

Impossible you can't generate sigs. There are many helper functions in util/ and util/crypto/ for you to use.
Again, cmd/playground mini program just said before.

ProofLocation will be proof message link and AltID will be UserID.

ProofLocation, kinda correct (if "message link" has the info you need when Validate()). AltID, not quite. See discussion in PR #45 for what AltID stands for.

@nykma
Copy link
Member

nykma commented Feb 25, 2023

I'M NOT A TEACHER CORRECTING YOUR HOMEWORK. REPRESENT YOUR RESULTS IN YOUR CODE.

I'll not respond to any homework-ish reply from now then.

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 25, 2023

🥲

@ash0521
Copy link
Contributor Author

ash0521 commented Feb 27, 2023

Please take a look now

Copy link
Member

@nykma nykma left a comment

Choose a reason for hiding this comment

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

Current code is OK to merge after CI turns green. Thank you for your contribution!

@nykma nykma merged commit 66a3974 into NextDotID:develop Feb 28, 2023
This was referenced Mar 2, 2023
Closed
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