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

Thumbs-up function #429

Closed
takahirom opened this issue Jan 19, 2020 · 22 comments · Fixed by #626
Closed

Thumbs-up function #429

takahirom opened this issue Jan 19, 2020 · 22 comments · Fixed by #626

Comments

@takahirom
Copy link
Member

takahirom commented Jan 19, 2020

Kind (Required)

  • Proposal / Discussion

Overview (Required)

  • We are showing thumbs-up instead of a session survey.
  • We have set up the necessary Firestore rules for it.
  • Please let us know if you have any missing rules or security issues.
rules_version = '2';
service cloud.firestore {
  match /databases/{database}/documents {
    match /confsched/2020 {
...
      match /sessions/{sessionId} {
        match /{allSubcollections=**} {
          allow create
        }
        // Allow to increment only the 'shards' field and only by 1.
        match /thumbsup_counters/{counterId} {
          allow get;
          allow write: if request.resource.data.keys() == ["shards"]
                         && (resource == null || request.resource.data.shards ==
                         resource.data.shards + 1);
        }
      }
    }
  }
}
  • I think that the user can press the button any number of times. (Cannot be canceled.) Like medium crap.
  • Regarding the creation of Firestore documents for sessions and counters, we are thinking of creating a Firestore document if there is no document from the application.

https://www.figma.com/file/4r9becvhDy3GfXaXex8E8d/App?node-id=21%3A289

now design
image image

Links

@mkeeda
Copy link
Contributor

mkeeda commented Jan 23, 2020

Can I challenge this issue ? 🙋

@takahirom
Copy link
Member Author

Thank you! Please ask if you have anything 🔥

@mkeeda
Copy link
Contributor

mkeeda commented Jan 26, 2020

@takahirom
I would like to avoid to post to database for release.
The firestore which specified android-base/arc/debug/google-services.json is test database for develop, isn't it?

@takahirom
Copy link
Member Author

@mkeeda Yes. That google-services.json is for develop 👍

@mkeeda
Copy link
Contributor

mkeeda commented Jan 26, 2020

memo

  • The app collects to thumbs-up count of each user.
    confsched/2020/sessions/${sessionId}/thumbsup_counters/${userId}/shards/${shardsId}
    • The app creates a thumbsup_counters document when a user taps a thumbs-up button at first, because no counter of the user.
  • Session Detail view show a thumbs-up count of a current user.
  • DroidKaigi staffs collect a thumbs-up count of all users of each session.

@takahirom
Copy link
Member Author

👀

@takahirom
Copy link
Member Author

@mkeeda
confsched/2020/sessions/${sessionId}/thumbsup_counters/${counterId}/shards/1
How about sharing data with other users in this way?

@mkeeda
Copy link
Contributor

mkeeda commented Jan 27, 2020

@takahirom
I'm thinking to share data with other users, but I don't understand what to mean /thumbsup_counters/${counterId}/.
Why is multiple counters in thumbs-up collection?
Are the single thumbs-up counter of each session some problems, like confsched/2020/sessions/${sessionId}/thumbsup_counter/shards/${shardId}?

@mkeeda
Copy link
Contributor

mkeeda commented Jan 27, 2020

Are the single thumbs-up counter of each session some problems, like confsched/2020/sessions/${sessionId}/thumbsup_counter/shards/${shardId}?

Sorry, I did not notice this document of firestore.

Notice the alternating pattern of collections and documents. Your collections and documents must always follow this pattern. You cannot reference a collection in a collection or a document in a document.
https://firebase.google.com/docs/firestore/data-model?hl=en

.../thumbsup_counter/shards/${shardId} is collection in collection; .../collection/collection/document, so it's wrong setting.

@takahirom
Copy link
Member Author

sessions/${sessionId}/thumbsup_counter/${shardId}
Probably we can remove like this?👀
I forgot why counterId is exists🤔🤔

@mkeeda
Copy link
Contributor

mkeeda commented Jan 27, 2020

@takahirom
Thank you🙏
I summarized the plans for sharing thumbs-up counts with all users.

Plan1: multiple counter

sessions/${sessionId}/thumbsup_counters/${userId}/shards/${shardsId}

  • The app sums up thumbs-up count of all users, and displays.
  • DroidKaigi staffs can check thumbs-up counts of each user. (not needed?)
  • Counting cost increases with number of user * number of shard.

Plan2: single counter

sessions/${sessionId}/thumbsup_counter/${shardId}

  • It's a simple implementation If counts of each user is not needed.
  • Counting cost increases with number of shard.
  • I still not check to be able implement this data model hierarchy.

@takahirom
Copy link
Member Author

takahirom commented Jan 27, 2020

I want to try Plan2.
Like this.

sessions/${sessionId}/thumbsup_counter/1
                   shards : 9
sessions/${sessionId}/thumbsup_counter/2
                   shards : 11
sessions/${sessionId}/thumbsup_counter/3
                   shards : 10

@mkeeda
Copy link
Contributor

mkeeda commented Jan 28, 2020

OK 🙆
I'll fix the code as soon as possible.

@mkeeda
Copy link
Contributor

mkeeda commented Jan 29, 2020

After tapped thumbs-up button, the app should update the thumbs-up count.
Can I use the realtime update function with firestore?
I think that the app is able to update automatically the count when other users tap thumbs-up button of the same session. 🤔

Get realtime updates with Cloud Firestore  |  Firebase
https://firebase.google.com/docs/firestore/query-data/listen?hl=en

And I want to implement extensions for Kotlin Coroutines Flow if I adopt the realtime update. 🤔🤔

@takahirom
Copy link
Member Author

I think so too. 👀
The favorite actually does the same thing except that there are multiple shards.
https://github.com/droidkaigi/conference-app-2020/blob/master/data/firestore/src/main/java/io/github/droidkaigi/confsched2020/data/firestore/internal/FirestoreImpl.kt#L38

@mkeeda
Copy link
Contributor

mkeeda commented Jan 29, 2020

Oh, sorry.
I missed it.😉
Thanks!!

@mkeeda
Copy link
Contributor

mkeeda commented Feb 2, 2020

@takahirom
I'm debugging to retrieve shards of thumbs-up counter.
I caught a permission error like this.

2020-02-02 03:05:28.280 4036-4106/io.github.droidkaigi.confsched2020.debug W/Firestore: (20.2.0) [Firestore]: Listen for Query(confsched/2020/sessions/155510/thumbsup_counter order by __name__) failed: Status{code=PERMISSION_DENIED, description=Missing or insufficient permissions., cause=null}
2020-02-02 03:05:28.499 4036-4036/io.github.droidkaigi.confsched2020.debug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: io.github.droidkaigi.confsched2020.debug, PID: 4036
    com.google.firebase.firestore.FirebaseFirestoreException: PERMISSION_DENIED: Missing or insufficient permissions.
        at com.google.firebase.firestore.util.Util.exceptionFromStatus(com.google.firebase:firebase-firestore@@20.2.0:121)
...

While, I probably successed to create shards.
Could you show the security rules of firestore after changing into the single counter way?

@takahirom
Copy link
Member Author

I will check today!

@mkeeda
Copy link
Contributor

mkeeda commented Feb 2, 2020

Security rules of /thumbsup_counters/{counterId} needs allow read.

  • The app has to retrieve all shards of thumbsup_counters but allow get allows only single document read requests.
  • allow list allows multiple document read requests.
  • allow read includes get and list

New rules↓
#626 (comment)

Discussion of changing rules ↓
https://twitter.com/mk_mkee/status/1223922969124040704?s=20

Thank you takahirom san!

@mkeeda
Copy link
Contributor

mkeeda commented Feb 4, 2020

When does the app show outlined button?

  1. When user has never tap thumbs-up button of the session.
  2. When thumbs-up count is zero

スクリーンショット 2020-02-04 17 26 25

Now the app show contained button if thumbs-up count is zero.
スクリーンショット 2020-02-04 17 48 27

@takahirom
Copy link
Member Author

takahirom commented Feb 4, 2020

I talked with designer 👍
0 = white
1 or more(all people) = blue.

@mkeeda
Copy link
Contributor

mkeeda commented Feb 5, 2020

Idea for reduce access to firestore

  • Change the security rule that allow only +1 update to thumbs-up count
    • Takahirom san already changed it!
request.resource.data.shards > resource.data.shards &&
request.resource.data.shards < resource.data.shards + 51
  • Only request to update the thumbs-up count if a particular timespan has passed
  • If users press the thumbs-up button continuously, only 1 request updates to firestore with a particular timespan
  • Control the stream using debounce operator like Rx
  • We can reduce number of shards too

Idea for users to feel repeated taps

  • Show the user's tapping count separately from the all count of the session
  • This idea is Inspired medium's crap

medium_crap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants