EAS-2497 : Collect possible GraphQL queries during lowering #56
Conversation
| @@ -99,7 +103,69 @@ impl fmt::Display for Error { | |||
| } | |||
| } | |||
|
|
|||
| impl std::error::Error for Error {} | |||
| fn check_graphql_and_perms(val: &Value) -> Vec<&str> { | |||
There was a problem hiding this comment.
What's the string being returned here? And does it need to be a string in the first place?
There was a problem hiding this comment.
This goes back to my internship, we decided on using string instead of enums for returning permissions.... I suppose we can swtich that. Let me know if we should.
There was a problem hiding this comment.
The main reason I'm going on this now, is because you have to hardcode the permissions anyway with the mapping, which means that the permissions are known at compile time.
Though if that's a stand in for deserializing an actual permission map where the permissions aren't static, then you can leave it as str, though make sure to add a comment.
There was a problem hiding this comment.
Yeah, i haven't completely decided on how I'll fetch them. I'll either be parsing the nadel files or just creating a map.... we can discuss this.
There was a problem hiding this comment.
Eventually, I want this to be enums, but I think we should make that change in a different task. Let me know if that sounds good :)
crates/fsrt/src/main.rs
Outdated
| .collect() | ||
| } | ||
|
|
||
| fn parse_graphql(s: &str) -> Vec<(&str, &str)> { |
There was a problem hiding this comment.
Could you add a comment explaining what the (&str, &str) element is?
Also, does this need to return a Vec or can you just return an impl Iterator<Item = (&str, &str)>, since we end up just passing it to an extend()?
There was a problem hiding this comment.
I think I resolved this ????
crates/fsrt/src/main.rs
Outdated
|
|
||
| // collect all fragments | ||
| if let std::result::Result::Ok(doc) = parse_query::<&str>(s) { | ||
| doc.definitions.clone().into_iter().for_each(|d| { |
There was a problem hiding this comment.
Could you add a TODO: use collect + avoid clone comment here? You can also do that in this PR(let me know if you need help), but it isn't super important to get it done in this PR.
There was a problem hiding this comment.
Fixed that ..... or so I hope
|
LGTM. Clean up the commit history, and I'll merge it in. |
gersbach
force-pushed
the
feat/EAS-2497
branch
from
ff82a3e to
9ce7343
Compare
No description provided.