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

#222 - activities crud #230

Merged
merged 15 commits into from
Apr 18, 2024
Merged

#222 - activities crud #230

merged 15 commits into from
Apr 18, 2024

Conversation

kamilpiech97
Copy link
Member

@kamilpiech97 kamilpiech97 commented Apr 10, 2024

Should close #222.

Added Activity model and CRUD views in admin panel. Some of activity fields are translatable via laravel-translatable.

Preview of create view:
image

@kamilpiech97 kamilpiech97 marked this pull request as ready for review April 10, 2024 09:20
@kamilpiech97 kamilpiech97 requested a review from a team as a code owner April 10, 2024 09:21
Copy link
Member

@EwelinaSkrzypacz EwelinaSkrzypacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. When I run php artisan migrate:fresh --seed I see error

image

As we talked with @kamilpiech97 we have to add empty folder activities

  1. Date on view/edit activity should be in Polish format (dd.mm.rrrr) - now it is like mm/dd/rrrr

image

  1. AFAIK the admin panel will be only in Polish, so we should show on index activities list title in Polish (now we show in English)

image

@kamilpiech97
Copy link
Member Author

  1. When I run php artisan migrate:fresh --seed I see error

image

As we talked with @kamilpiech97 we have to add empty folder activities

  1. Date on view/edit activity should be in Polish format (dd.mm.rrrr) - now it is like mm/dd/rrrr

image

  1. AFAIK the admin panel will be only in Polish, so we should show on index activities list title in Polish (now we show in English)

image

1, 2. fixed
3. should be done in next PR, filament has some connected stuff with translating and nice will be do this in new PR

Copy link
Member

@EwelinaSkrzypacz EwelinaSkrzypacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me date is still in format mm/dd/yyyy

image

@EwelinaSkrzypacz
Copy link
Member

With @kamilpiech97 we decided to leave dates to next pr - on some browser works (Kamil has Google Chrome in English and date display is in format dd.mm.yyyy, but on mine browser - Google Chrome in English also - I have format mm/dd/rrrr)

Copy link
Member

@EwelinaSkrzypacz EwelinaSkrzypacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Filters for dates doesn't work - for example I created an activity with publish date 11.04.2024 and I choose filters from 08.04.2024 to 08.04.2024, so I shouldn't see the activity, but it is

image

  1. I think we should add sorting - by date asc and desc?

  2. "The data publikacji field is required unless opublikowane is in 1." - this is validation message, when user check "Opublikowane" but doesn't fill date - can we write this somehow better? Like "Pole data publikacji jest wymagane, jeżeli została zaznaczona opcja Opublikowane" or something like that. We can also leave this to pr related to translations, it can be treated as reminder.

  3. Can we add search input?

@kamilpiech97
Copy link
Member Author

  1. Filters for dates doesn't work - for example I created an activity with publish date 11.04.2024 and I choose filters from 08.04.2024 to 08.04.2024, so I shouldn't see the activity, but it is

image

  1. I think we should add sorting - by date asc and desc?
  2. "The data publikacji field is required unless opublikowane is in 1." - this is validation message, when user check "Opublikowane" but doesn't fill date - can we write this somehow better? Like "Pole data publikacji jest wymagane, jeżeli została zaznaczona opcja Opublikowane" or something like that. We can also leave this to pr related to translations, it can be treated as reminder.
  3. Can we add search input?

1, 2. fixed
3. I will do it in language PR or connected PR with i18n stuff
4. added

Copy link
Member

@EwelinaSkrzypacz EwelinaSkrzypacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kamilpiech97 kamilpiech97 merged commit d0a6c2c into main Apr 18, 2024
2 checks passed
@kamilpiech97 kamilpiech97 deleted the #222-activities-crud branch April 18, 2024 11:02
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.

[Dashboard] Activities
3 participants