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

Feature Request: Using proper graphql scalars for different data types shared between clients and server #1559

Closed
xoldd opened this issue Feb 23, 2023 · 35 comments · Fixed by #1598 or #1692
Assignees
Labels
feature request no-issue-activity No issue activity test Testing application

Comments

@xoldd
Copy link

xoldd commented Feb 23, 2023

Is your feature request related to a problem? Please describe.

Talawa-api currently uses default scalars provided by graphql to parse/validate queries and exchange data with the clients. We are migrating to making use of graphql-scalars package and custom scalar types in talawa-api. Check out these links for more info:- link1, link2.

You can work with eshaan who's working on this in talawa-api to figure stuff out.

Describe the solution you'd like

We'd like talawa mobile to migrate their graphql SDL documents to make use of the new scalars. This would provide better validation to fields and a certain standard to the data exchanged between talawa mobile and talawa api.

Describe alternatives you've considered

Approach to be followed

  1. Replace the old scalar values with the new ones in the main app. This would probably include the functionality of receiving the new scalar type values from the api and parsing/transforming them to whatever the data type requirements are in dart/flutter and the app.
  2. Migrate your test code to make use of the new scalar values. In the tests make sure you're not using dummy data which doesn't conform to new scalars as later it could prove fatal if api is used in any way for E2E tests or something of the sort.

Additional context

Potential internship candidates

@github-actions github-actions bot added test Testing application unapproved Unapproved, needs to be triaged labels Feb 23, 2023
@literalEval
Copy link
Member

Seems like an important migration. @palisadoes @noman2002 can you please assign this to me as I am working on migration issues these days.

@xoldd xoldd removed the unapproved Unapproved, needs to be triaged label Feb 24, 2023
@xoldd
Copy link
Author

xoldd commented Feb 24, 2023

Seems like an important migration. @palisadoes @noman2002 can you please assign this to me as I am working on migration issues these days.

done

@EshaanAgg
Copy link

@literalEval In addition to migrating the SDL definitions to the new scalars, you would also need to check compatibility for date objects as ISO 8601 standard, instead of the earlier UNIX Timestamp standard.
Details about the same can be found in this PR

@literalEval
Copy link
Member

Noted

@noman2002
Copy link
Member

@literalEval I have to unassign this. It will unfair to others. You are 4 issues in your name.

@noman2002 noman2002 added the unapproved Unapproved, needs to be triaged label Feb 24, 2023
@literalEval
Copy link
Member

literalEval commented Feb 24, 2023

@noman2002 the two issues related to Custom Lint are literally the same issue. One of them is just for tracking purposes. I have said this earlier too. That issue will take some time as currently I am looking for someone who can test that feature with me. In the meantime I can work on this. Please assign this to me :(

@xoldd xoldd added points 05 and removed points 02 labels Feb 24, 2023
@xoldd
Copy link
Author

xoldd commented Feb 24, 2023

@noman2002 @literalEval

Hehe this is a bit problematic. I'd like people to start working on this refactor asap. Btw it shouldn't be limited to only one person, many people can collab to work on this dividing up work.

@literalEval
Copy link
Member

literalEval commented Feb 24, 2023

:) assign this to me. I'm waiting :)

@noman2002
Copy link
Member

@noman2002 the two issues related to Custom Lint are literally the same issue. One of them is just for tracking purposes. I have said this earlier too. That issue will take some time as currently I am looking for someone who can test that feature with me. In the meantime I can work on this. Please assign this to me :(

Things don't work this way. The custom lint issue is huge. You cannot work on both at the same time. Let others try this. Complete the custom lint issue get it merged then I'll be more than happy to assign.

@noman2002
Copy link
Member

noman2002 commented Feb 24, 2023

If both the issues are same then why are we having two issues. Close any one of them. Then I'll assign this.

@literalEval
Copy link
Member

literalEval commented Feb 24, 2023

@noman2002 the two issues related to Custom Lint are literally the same issue. One of them is just for tracking purposes. I have said this earlier too. That issue will take some time as currently I am looking for someone who can test that feature with me. In the meantime I can work on this. Please assign this to me :(

Things don't work this way. The custom lint issue is huge. You cannot work on both at the same time. Let others try this. Complete the custom lint issue get it merged then I'll be more than happy to assign.

Yes that is a huge issue but it will inevitably take quite some time as I have mentioned above

If both the issues are same then why are we having two issues. Close any one of them. Then I'll assign this.

Because I couldn't modify the original one to track progress. So I created another one. If you feel like it is not worth having, I can close the one I have created for tracking.

I am just saying that I am idle and would like to work on something :)

@literalEval
Copy link
Member

@noman2002 I have closed one of them :)

@noman2002 noman2002 removed the unapproved Unapproved, needs to be triaged label Feb 24, 2023
@xoldd
Copy link
Author

xoldd commented Feb 27, 2023

@literalEval are u working on this? This is important as api and admin have already made all the changes and the changes are merged. If mobile is not migrated quickly many contributers will get issues.

@literalEval
Copy link
Member

@xoldyckk Yes I am working on this on priority basis and am in continuous contact with @EshaanAgg

@literalEval
Copy link
Member

@xoldyckk I have migrated for the event system. Migration for Posts remains.

@literalEval
Copy link
Member

Since Event is the most used feature (as it is fetched/displayed by default), should we merge the work done till now and deal with Post in another PR ? This will at least reduce the error frequency and then I will migrate Post system by tomorrow ?

@xoldd
Copy link
Author

xoldd commented Feb 28, 2023

@literalEval You can create many small PRs. Just reference this issue in them so they'll be easier to track cuz they are sub PRs for a bigger issue.

Also, I'm clueless about dart/flutter so your PRs won't be reviewed by me. So, I can't say anything about them getting merged.

@Ayush0Chaudhary
Copy link

Ayush0Chaudhary commented Mar 13, 2023

Since Event is the most used feature (as it is fetched/displayed by default), should we merge the work done till now and deal with Post in another PR ? This will at least reduce the error frequency and then I will migrate Post system by tomorrow ?

@literalEval can i also work on this alongside you? I can work on sub parts as you mentioned above.

@noman2002 @palisadoes @xoldyckk

@literalEval
Copy link
Member

literalEval commented Mar 13, 2023

@Ayush0Chaudhary Sure we can pair up. The posts PR just got merged today. We can look into that.

@Ayush0Chaudhary
Copy link

corrected!

@github-actions github-actions bot removed the no-issue-activity No issue activity label Mar 14, 2023
Ayush0Chaudhary added a commit to Ayush0Chaudhary/talawa that referenced this issue Mar 20, 2023
@github-actions
Copy link

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Mar 24, 2023
@palisadoes
Copy link
Contributor

@literalEval Are you still working on this?

@literalEval
Copy link
Member

Yeah @palisadoes I and @Ayush0Chaudhary both are working on this. We decided to pair up as this is a very important and large issue. Thanks.

@github-actions github-actions bot removed the no-issue-activity No issue activity label Mar 25, 2023
palisadoes pushed a commit that referenced this issue Mar 26, 2023
* Fixes #1559 Fixes the fetch post

* Added Documentation

* Added docs
@Ayush0Chaudhary
Copy link

@palisadoes other services are still left to be corrected, we should reopen this.

@palisadoes palisadoes reopened this Mar 26, 2023
@palisadoes
Copy link
Contributor

  1. What services are left?
  2. Why don't we create individual issues for them?

We can add check boxes to this issue to keep track.

We have reopened this issue multiple times. That's not a suitable way to operate.

@Ayush0Chaudhary
Copy link

Ayush0Chaudhary commented Mar 29, 2023

  1. What services are left?
  2. Why don't we create individual issues for them?

We can add check boxes to this issue to keep track.

We have reopened this issue multiple times. That's not a suitable way to operate.

@palisadoes @noman2002 @xoldyckk

  • Direct Service
  • Comment Services
  • User Config

These are only few that I know of, others are still unidentified.

@github-actions
Copy link

github-actions bot commented Apr 9, 2023

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Apr 9, 2023
@literalEval
Copy link
Member

  1. What services are left?
  2. Why don't we create individual issues for them?

We can add check boxes to this issue to keep track.
We have reopened this issue multiple times. That's not a suitable way to operate.

@palisadoes @noman2002 @xoldyckk

  • Direct Service
  • Comment Services
  • User Config

These are only few that I know of, others are still unidentified.

@Ayush0Chaudhary are you working on these? Or should I start from my end?

@Ayush0Chaudhary
Copy link

For now you can leave them, I will address them in other issue which Target all similar shortcomings.

@github-actions github-actions bot removed the no-issue-activity No issue activity label Apr 10, 2023
@github-actions
Copy link

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Apr 21, 2023
@palisadoes
Copy link
Contributor

What's the status on this?

@github-actions github-actions bot removed the no-issue-activity No issue activity label Oct 14, 2023
@github-actions
Copy link

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment