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

Parse DID correctly #14

Closed
wants to merge 1 commit into from
Closed

Parse DID correctly #14

wants to merge 1 commit into from

Conversation

cobanov
Copy link

@cobanov cobanov commented Apr 24, 2023

Previously, the code was not parsed the DID response correctly, resulting in incorrect data being parsed. This commit fixes the issue by properly splitting and parsing the DID.

Previously, the code was not parsed the DID response correctly, resulting in incorrect data being parsed. This commit fixes the issue by properly splitting and parsing the DID.
@ianklatzco
Copy link
Owner

taking a look.....

@ianklatzco
Copy link
Owner

This change causes 2 tests to fail:

F....F..
======================================================================
FAIL: test_follow (tests.test_main.TestSessionLogin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/atprototools/tests/test_main.py", line 35, in test_follow
    self.assertEqual(resp.status_code, 200)
AssertionError: 400 != 200

======================================================================
FAIL: test_post_skoot (tests.test_main.TestSessionLogin)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/atprototools/tests/test_main.py", line 54, in test_post_skoot
    self.assertEqual(resp.status_code, 200)
AssertionError: 400 != 200

----------------------------------------------------------------------
Ran 8 tests in 7.731s

FAILED (failures=2)

In test_follow and follow,

data = {
"collection": "app.bsky.graph.follow",
"repo": "{}".format(self.DID),
"record": {
"subject": did_of_person_you_wanna_follow,

The record is expected to contain "repo": "did:plc:blahblahblah"

Splitting the DID sends only blahblahblah.

It might be better to manually put did:plc in follow, but I'm not sure.

What do you think? ^^

(thanks for the PR!)

@cobanov
Copy link
Author

cobanov commented Apr 25, 2023

as you can see here each function which using 'DID' expecting in that format, what do you think maybe we can init another attribute self.DID_key maybe?

https://gist.github.com/cobanov/023f497e6790b74e79c44c76ce67e68b

@ianklatzco
Copy link
Owner

i think it might be better to have the user manually do a session.DID.split(":")[-1] or add did:plc: back whenever they need to ^^

although, atprototools is a bit inconsistent in some places (e.g. usage of did vs username, which_case theCodeIsUsing). that would be a good thing to look at ^^

@ianklatzco ianklatzco closed this May 2, 2023
@ianklatzco
Copy link
Owner

here's the related issue ^^ #12

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.

2 participants