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

feat: add journal entry edit option #2802

Merged
merged 19 commits into from
Aug 23, 2019

Conversation

TeddyBear06
Copy link

@TeddyBear06 TeddyBear06 commented Jun 26, 2019

Hi,

Before spending time on writing tests, I would like to get your opinion about this feature.

It is my first attempt to contribute to this project, so I probably need to adapt :-)

Checklist

Before submitting the PR

General checks

  • Make sure that the change you propose is the smallest possible.
  • The name of the PR should follow the conventional commits guideline that the project follows.

Front-end changes

  • If you change the UI, make sure to ask repositories administrators first about your changes by pinging @djaiss or @asbiin in this PR.
  • Screenshots are included if the PR changes the UI.
    oLRvF62
    db23p7U
  • Front-end tests have been written with Cypress.

Backend/models changes

  • The API has been updated.
  • API's documentation has been added by submitting a pull request in the marketing website repository.
  • Tests have been added for the new code.
  • If you change a model, make sure the FakeContentTableSeeder is updated. We need seeders to develop locally and generate fake data.

If the code changes the SQL schema

  • Make sure exporting account data as SQL is still working.
  • Make sure your changes do not break importing data with vCard and .csv files.
  • Make sure account reset and deletion still work.

Other tasks

  • CHANGELOG entry added, if necessary, under UNRELEASED.
  • CONTRIBUTORS entry added, if necessary.
  • If it's relevant and worth mentioning, create a changelog entry for this change. The changelog entry will appear inside the UI for all users to see. To know if your change is worth the creation of a changelog entry, read the documentation.
  • Don't forget to ask for a free account on https://monicahq.com as anyone who contributes can request a free account.

@TeddyBear06 TeddyBear06 changed the title [wip] 818 add journal entry edit option 818 add journal entry edit option Jul 1, 2019
@TeddyBear06 TeddyBear06 changed the title 818 add journal entry edit option [wip] 818 add journal entry edit option Jul 1, 2019
Copy link
Member

@asbiin asbiin left a comment

Choose a reason for hiding this comment

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

It looks very good !
Some little changes to do, but it's ok for me.

resources/lang/fr/journal.php Outdated Show resolved Hide resolved
app/Models/Journal/JournalEntry.php Outdated Show resolved Hide resolved
resources/views/journal/edit.blade.php Outdated Show resolved Hide resolved
routes/web.php Outdated Show resolved Hide resolved
@TeddyBear06
Copy link
Author

Updated :-)

Let me know if you want anything else.

Kind regards,

@asbiin asbiin changed the title [wip] 818 add journal entry edit option feat: add journal entry edit option Aug 23, 2019
@asbiin asbiin merged commit 7e5a5a3 into monicahq:master Aug 23, 2019
@asbiin
Copy link
Member

asbiin commented Aug 23, 2019

@TeddyBear06 thanks a lot! As you can see I've updated some of your code, for the better.

@TeddyBear06
Copy link
Author

@asbiin Wonderful! Thanks a lot! I think this little feature will be great :-)

@danielsundermeier
Copy link

i would suggest a little change in /Users/daniel/code/monicahq/monica/resources/assets/js/components/journal/partials/JournalContentEntry.vue:38

  • :href="'/journal/entries/' + entry.id + '/edit'"
  • :href="'journal/entries/' + entry.id + '/edit'"

right now it does not work if monica is not installed in the root directory

(i don't know how to do it myself)

@asbiin
Copy link
Member

asbiin commented Aug 28, 2019

@danielsundermeier thanks for the feedback ! it's fixed.

@TeddyBear06
Copy link
Author

@danielsundermeier Good catch! @asbiin Thanks!

@github-actions
Copy link

This pull request has been automatically locked since there
has not been any recent activity after it was closed.
Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants