dwyl / calendar

📅 effortlessly know when everyone in your team is available / busy with a single glance.
https://gcal.fly.dev/
GNU General Public License v2.0
27 stars 1 forks source link

[PR] Initial implementation of `calendar` #25

Closed LuchoTurtle closed 1 year ago

LuchoTurtle commented 1 year ago

closes #21 closes #27

Creates the initial implementation of the calendar app, which lists the events of a logged-in person and allows the same to create a new one.

nelsonic commented 1 year ago

@LuchoTurtle this is looking good so far. How are you getting on today? 🧑‍💻 👌

LuchoTurtle commented 1 year ago

Having some trouble with getting information from the API. I'm trying to list calendars from myself but I keep getting "insufficient authentication scopes" from the API. I'm going through some links I found and if anything works, I'll document it on the BUILDIT.md file I'm writing on.

LuchoTurtle commented 1 year ago

This is taking much longer than I wanted to, mostly because of scopes. I kept getting Request had insufficient authentication scopes. every time I wanted to get a token from Google so I could use it to fetch information from the Google Calendar API.

After battling through this, I noticed that even following the guide from https://github.com/dwyl/elixir-auth-google#optional-scopes, the package always requested scope: profile and ignored my attempts to add calendar-related ones.

After trying everything I could, I tried to reset the project and start anew and noticed it was a problem when I tried to override the scopes through the .env file. If I do something like export GOOGLE_SCOPE=profile email calendar, only profile is considered and the rest is ignored.

After I reset the env variables and did the overriding through config.ex, I was able to surpass the error but couldn't get my account verified. I then had to add the scopes inside Google Cloud and add my e-mail as test users so I could actually get to the consent screen and get information from the API. I documented these steps in BUILDIT.md.

I've created an issue in https://github.com/dwyl/elixir-auth-google/issues/92 so elixir-auth-google can get a bit of an update to fix the README after this issue is done with.

nelsonic commented 1 year ago

@LuchoTurtle after figuring out that the GOOGLE_SCOPE environment variable had to be a (quoted) String, were you able to make progress with accessing the Calendar? 🤞

nelsonic commented 1 year ago

Please leave updates in the issue https://github.com/dwyl/calendar/issues/21 especially when something isn't working as you expect it. 🙏

LuchoTurtle commented 1 year ago

@LuchoTurtle after figuring out that the GOOGLE_SCOPE environment variable had to be a (quoted) String, were you able to make progress with accessing the Calendar? 🤞

I did make progress because the scopes were being properly sent, yes. Although I reset my env variables and just went with the config.ex route instead because it's more explicit in the code.

LuchoTurtle commented 1 year ago

Fetching data from the API should now be working. I'll focus on functionality and showing this data to the user.

LuchoTurtle commented 1 year ago

Items are now being shown to the user person, like so:

list_items1

nelsonic commented 1 year ago

@LuchoTurtle looks like progress. thanks for sharing. 👌

LuchoTurtle commented 1 year ago

Having some problems with the code from https://tailwindcomponents.com/component/calendar-ui-with-tailwindcss-and-alpinejs.

I've successfully imported the code and made the necessary changes to make it work with Phoenix (by following the steps in https://github.com/phoenixframework/phoenix_live_view/issues/2049).

Since the example is using Alpine.js, I've added the <script> to install it in root.html.heex and changed assets/js/app.js to have Alpine.js work properly.

    dom: {
        onBeforeElUpdated(from, to) {
          if (from._x_dataStack) {
            window.Alpine.clone(from, to)
          }
        }
    },

However, the model to add a new event is shown when the page loads, when it isn't supposed to. I've tried using x-cloak to solve this issue and it does to an extent. The issue is that after loading, the calendar disappears completely... I'm sure I'm using x-cloak properly but it seems to be battling against me :/

nelsonic commented 1 year ago

@LuchoTurtle if you can avoid using Alpine.js please just skip it and stick with TailwindCSS.

LuchoTurtle commented 1 year ago

I want to give a status report on using the code borrowed from https://tailwindcomponents.com/component/calendar-ui-with-tailwindcss-and-alpinejs and trying to add it to the calendar so the user person can:

The issue

https://user-images.githubusercontent.com/17494745/233591283-9be1eb0d-37f4-4ff8-880f-522a3ddbeb35.mov

When I sign in and am redirected to the /app URL, the calendar is correctly rendered. However, this only occurs for a second and it's "collapsed". The calendar is rendered back again when I try to navigate the months.

This can only be due to two things:

The reason I have a feeling the issue is related to the template code in app_live.html.heex is because the calendar is only collapsed when the socket assigns are changed. Look at the video below:

https://user-images.githubusercontent.com/17494745/233593297-9a7d1015-16d0-452b-a0b8-44cc4167a6af.mov

When I click on days that have no events (the events are stored in the socket assigns in an array called event_list), the calendar is not collapsed. However, when event_list effectively changes, the calendar collapses.

This behaviour is extremely weird. I figure that some function that was defined in the <script> tag from the code we borrowed is acting weird because of this socket change, even though it shouldn't have problems with it.

I'll continue researching but I get to a dead-end, I might convert this calendar to a server-side rendered object and try to surpass this issue with this approach.

LuchoTurtle commented 1 year ago

I think this issue was fixed through https://github.com/phoenixframework/phoenix_live_view/issues/809#issuecomment-637671803.

Went from:

 dom: {
        onBeforeElUpdated(from, to) {
          if (from._x_dataStack) {
            window.Alpine.clone(from, to)
          }
        }
    },

To:

    dom: {
        onBeforeElUpdated(from, to){
          if(from.__x){ window.Alpine.clone(from.__x, to) }
        }
    },
nelsonic commented 1 year ago

@LuchoTurtle I feel like we're working way too hard to get this ... 🧑‍💻 😓 I really appreciate your efforts, you appear to be going the extra mile, however it's already way beyond the scope of the SPIKE https://github.com/dwyl/calendar/issues/21 ... ⏳

You must learn to time-box your efforts and keep things deliberately simple. Perhaps I should have been [a lot] more specific with the requirement to "Display them [events] on a page" ... 💭 For future reference, when there isn't a specific requirement for UI to be "polished", then keep things very basic. A Text-only interface is "enough".

I appreciate that you borrowed code from https://tailwindcomponents.com/component/calendar-ui-with-tailwindcss-and-alpinejs which includes Apline.js ... That wasn't one of the ones mentioned in https://github.com/dwyl/calendar/issues/27 💭

It uses an "old" version of both Tailwind and Alpine.js which should only ever be used as a reference ...

calendar-uses-ancient-tailwind

The idea of borrowing "free" code is to speed up development ... 🤞 But, I'm sure you'll agree, you've reached the point where it's slowing you down. 🐢 Building a Calendar using LiveView definitely doesn't need Alpine.js ... 🙅 The state should all be managed by Phoenix / LiveView and rendered by LiveView components. Using Tailwind to make the interface look good is very much a "stretch goal" of this ... Should we just pay for Tailwind UI https://github.com/dwyl/learn-tailwind/issues/67 and move on to the interesting part of this project? 💭

LuchoTurtle commented 1 year ago

You're right but the problem with the Tailwind Calendar has fortunately been surpassed. I'm currently working on LiveView to get the POST operation properly working, with some basic form validation.

The reason I went with the linked calendar and forewent the links you mentioned in #27 is because the links you've mentioned were either:

The one I chose, even though was made with an earlier version of Tailwind, fortunately used classes that are compatible with the latest version, hence why it now actually works.

I'm working on getting POST operations working and not all state lives in the LiveView currently, as you've mentioned today on standup. However, I'm focused on finishing the rest of the features to close of #21 (and #27 simultaneously) and get a working version with all the steps properly documented.

If migrating the calendar state to LiveView is a must, I believe it could be done on a separate PR or on this one but after a basic version is effectively working. 👍

LuchoTurtle commented 1 year ago

The project should work as intended according to what was asked in #21. However, I want to simplify the code a bit and simplify the homepage so it goes in tandem to what was asked in #21.

There's also a small issue with the dates when fetching the event list but it should be easily surpassable, as per https://stackoverflow.com/questions/24972240/google-calendar-api-querying-all-day-events.

LuchoTurtle commented 1 year ago

All the necessary documentation should be complete.

Tests were added that cover 100% of the code. Some bugs were fixed (querying events wasn't working at 100% due to date times being malformed without the timezone) and the code was refactored/simplified.

Not marking this for review because I want to add the CI before that.

nelsonic commented 1 year ago

@LuchoTurtle as discussed on standup, you're going to get the GitHub Actions CI/CD working on this and do a final review of the copy and .then assign to me for review. 👌

LuchoTurtle commented 1 year ago

@nelsonic this should be done. For the application to successfully deploy, GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET need to be set in the CI when building the Phoenix project. I'm going to let you create the project under dwyl and not under my person account. The steps for this should be present in the README/BUILDIT file.

You will need to add the values of these two env variables to the secrets of this repo (in the Settings tab) so the deploy.yml workflow works.

codecov[bot] commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@88bc996). Click here to learn what that means. The diff coverage is n/a.

@@           Coverage Diff            @@
##             main       #25   +/-   ##
========================================
  Coverage        ?   100.00%           
========================================
  Files           ?         8           
  Lines           ?        82           
  Branches        ?         0           
========================================
  Hits            ?        82           
  Misses          ?         0           
  Partials        ?         0           
nelsonic commented 1 year ago

@LuchoTurtle thanks very much for the update and assigning for review. 👌

nelsonic commented 1 year ago

If you see an error when attempting to authorise e.g:

image

Retrace your steps to setup the API keys correctly. 🔙