Add #381 Add hash ids instead of actual ID for model routes [WIP] #777

turtles2 · 2018-01-08T18:15:29Z

This will close #381 . Its current state its a proof of concept it needs the following things which I will work on.

Extend to all Models
Add Tests for added functions
Modify Tests to Work with new URL structure
Allow both old and new URLs to work
Add HashIDs to URLs that don't use binding (Add ones from review)

Pull from Main

Add webmanifest (monicahq#726)

Change to git clone URL as the git@ will not work without a granted private key.

…icahq#731) When the current day of the month is greater than the number of days in the month, the date class was rolling forward to March.

…ahq#735) This will allow a user to search there contacts by there contact information. It is done by entering the field they are searching by followed by a : then the search term. For example to find a contact with the email [email protected] you would enter email:[email protected]. Fixes monicahq#729

Merge Upstream

Revert "Fix URL::forceSchema to forceScheme (monicahq#730)" (monicahq#744)

Merge Remote

asbiin · 2018-01-08T18:18:49Z

app/Helpers/ID_hasher.php

+
+use Vinkla\Hashids\Facades\Hashids;
+
+class ID_hasher


Rename class "ID_hasher" to match the regular expression ^[A-Z][a-zA-Z0-9]*$.

The name ID_hasher is not PSR1 compliant.

Please change it everywhere with idHasher.

djaiss · 2018-01-08T19:06:57Z

@turtles2 instead of writing Add #381 in the title, could you write a description of what the PR does? It'll be easier. Thanks! And great for the feature you are trying to implement!

turtles2 · 2018-01-10T15:23:40Z

@djaiss I am unsure on the best way to handle the existing tests. Many of them right now (18) use the model ID in there URL structure that will continue to work. But do we want to change them to use the hashid instead? At first I was thinking that we do but now I think it would be best to leave them as is and only test hashid for the sake of testing hashids.

turtles2 · 2018-04-02T02:29:29Z

@djaiss I have added the hashIDs to the new relationships but I am getting an error "global_relationship_form_new_contact is not defined" on the editing page. I not sure why I am getting this. Besides this error it is ready for merging.

djaiss · 2018-04-02T14:41:09Z

@turtles2 yes, this error is due to this variable missing in app.js.

To fix it, until a proper fix arrives, you can do this at the bottom of app.js:

loadLanguageAsync(window.Laravel.locale, true).then((lang) => {

    // the Vue appplication
    me.app = new Vue({
      i18n,
      data: {
        activities_description_show: false,
        reminders_frequency: 'once',
        accept_invite_user: false,
        date_met_the_contact: 'known',
        global_relationship_form_new_contact: true,   ←-----------------HERE
      },
      methods: {
      },
      mounted: function() {

        // required modules
        require('./tags');
        require('./search');
        require('./contacts');

      }
    }).$mount('#app');

    return app;
});

and recompile the assets (npm run prod) and it should work.

asbiin · 2018-04-03T22:26:07Z

resources/views/people/debt/index.blade.php

@@ -46,7 +46,7 @@
          @endif
        </div>
        <div class="table-cell list-actions">
-          <a href="{{ route('people.debt.edit', ['people' => $contact->id, 'debtId' => $debt->id]) }}">
+          <a href="{{ route('people.debt.edit', [$contact, $debt]) }}">


Why not keep the named attributes here ?

@asbiin I would have to test it to be sure but it's one of two reasons. Either there was some quirk in laravels route binding that required it, or I got carried away with find and replace. Looking at I would lean towards the first one.

@turtles2 indeed ! The previous version was obviously buggy. Thanks !

asbiin · 2018-04-04T17:59:52Z

We have just switched to Laravel 5.6, so I've updated vinkla/hashids dependency to '^5.0'

asbiin · 2018-04-04T17:56:08Z

app/Helpers/IdHasher.php

+
+    public function decodeId($hash)
+    {
+        if (str_contains($hash, $this->prefix)) {


Would it not be better to call starts_with ?

@asbiin Yes it would be. I was not aware of that function.

turtles2 · 2018-04-07T12:48:06Z

I will take a look tonight and if that's the case hopefully fix it as well.

…

On Sat, Apr 7, 2018, 3:32 AM Alexis Saettler ***@***.***> wrote: Contact deletion seems to be broken, can you confirm ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#777 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADfAuuzVv3txFV1-zdTGStIrNUl6PVk1ks5tmHm4gaJpZM4RWwP2> .

asbiin · 2018-04-07T15:09:51Z

@turtles2 It was related to another issue, fixed in #1114

djaiss · 2018-04-08T18:19:26Z

@turtles2 thanks so much for your patience with this PR. I'm sorry it took so long to merge it.

This is an awesome work.

turtles2 · 2018-04-08T18:23:07Z

No worries with changes like this we need to get it right before we ship

…

On Sun, Apr 8, 2018, 1:19 PM Régis Freyd ***@***.***> wrote: @turtles2 <https://github.com/turtles2> thanks so much for your patience with this PR. I'm sorry it took so long to merge it. This is an awesome work. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#777 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADfAurK4GahRex1fHdOd8w554B7EOTwkks5tmlSxgaJpZM4RWwP2> .

github-actions · 2021-02-11T00:11:58Z

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.

turtles2 and others added 16 commits December 25, 2017 19:27

add DB_PREFIX

e7ed9a7

Update database.php

30c44c5

Merge pull request #1 from monicahq/master

6f8aae1

Pull from Main

Clean Up Commit

19039e6

Merge pull request #2 from monicahq/master

c780dd7

Add webmanifest (monicahq#726)

Fixes monicahq#350

7081fbe

Merge branch 'master' into master

d04ee9a

Bump version to 1.5.0 (monicahq#737)

08077a4

Change link to repository in the documentation (monicahq#728)

32a2a04

Change to git clone URL as the git@ will not work without a granted private key.

Fix URL::forceSchema to forceScheme (monicahq#730)

8a803e7

Fix an issue where February shows up as March in the date picker (mon…

0d842ec

…icahq#731) When the current day of the month is greater than the number of days in the month, the date class was rolling forward to March.

Merge pull request #4 from monicahq/master

1c266d4

Merge Upstream

Merge pull request #5 from monicahq/master

74d3251

Revert "Fix URL::forceSchema to forceScheme (monicahq#730)" (monicahq#744)

Merge pull request #6 from monicahq/master

68cb57e

Merge Remote

Proof of Concept

6710767

asbiin reviewed Jan 8, 2018

View reviewed changes

turtles2 changed the title ~~Add #381 [WIP]~~ Add #381 Add hash ids instead of actual ID for contacts [WIP] Jan 8, 2018

turtles2 changed the title ~~Add #381 Add hash ids instead of actual ID for contacts [WIP]~~ Add #381 Add hash ids instead of actual ID for model routes [WIP] Jan 8, 2018

turtles2 and others added 7 commits January 8, 2018 15:23

Support for URLs that don't use HashIDs

efaf6db

Merge branch 'master' into 381

dd6c55f

Composer Fix

b223e31

Allow setting of salt

1a7a590

Test

2ce2a03

Kid Fix

1ea2a03

Merge branch 'master' into 381

ea9220a

turtles2 added 2 commits January 10, 2018 09:46

Fix Kid Issue

c876f0d

Add tests

f214e74

Merge branch 'master' into 381

a01af38

asbiin temporarily deployed to monica-team-pr-777 April 2, 2018 01:36 Inactive

turtles2 added 2 commits April 1, 2018 21:13

Fixes

a4eb62d

Merge branch '381' of https://github.com/turtles2/monica into 381

6f037eb

asbiin temporarily deployed to monica-team-pr-777 April 2, 2018 02:14 Inactive

djaiss approved these changes Apr 3, 2018

View reviewed changes

asbiin requested changes Apr 3, 2018

View reviewed changes

asbiin approved these changes Apr 4, 2018

View reviewed changes

Merge remote-tracking branch 'origin/master' into 381

a5c65c7

asbiin temporarily deployed to monica-team-pr-777 April 4, 2018 17:59 Inactive

asbiin requested changes Apr 5, 2018

View reviewed changes

Update IdHasher.php

f311fe3

asbiin temporarily deployed to monica-team-pr-777 April 5, 2018 13:46 Inactive

asbiin approved these changes Apr 5, 2018

View reviewed changes

Merge remote-tracking branch 'origin/master' into 381

975dd79

asbiin temporarily deployed to monica-team-pr-777 April 7, 2018 08:32 Inactive

Merge branch 'master' into 381

8aadf00

asbiin deployed to monica-team-pr-777 April 7, 2018 22:47 View deployment

djaiss merged commit 3ba96cf into monicahq:master Apr 8, 2018

turtles2 deleted the 381 branch April 9, 2018 02:21

turtles2 mentioned this pull request Jun 3, 2018

Avatar in 'Journal' page links to nowhere #1438

Closed

github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add #381 Add hash ids instead of actual ID for model routes [WIP] #777

Add #381 Add hash ids instead of actual ID for model routes [WIP] #777

turtles2 commented Jan 8, 2018 •

edited

Loading

asbiin Jan 8, 2018

djaiss Jan 28, 2018

djaiss commented Jan 8, 2018

turtles2 commented Jan 10, 2018

turtles2 commented Apr 2, 2018

djaiss commented Apr 2, 2018

asbiin Apr 3, 2018

turtles2 Apr 4, 2018

asbiin Apr 4, 2018

asbiin commented Apr 4, 2018 •

edited

Loading

asbiin Apr 4, 2018

turtles2 Apr 5, 2018

turtles2 commented Apr 7, 2018 via email

asbiin commented Apr 7, 2018

djaiss commented Apr 8, 2018

turtles2 commented Apr 8, 2018 via email

github-actions bot commented Feb 11, 2021

Add #381 Add hash ids instead of actual ID for model routes [WIP] #777

Add #381 Add hash ids instead of actual ID for model routes [WIP] #777

Conversation

turtles2 commented Jan 8, 2018 • edited Loading

asbiin Jan 8, 2018

Choose a reason for hiding this comment

djaiss Jan 28, 2018

Choose a reason for hiding this comment

djaiss commented Jan 8, 2018

turtles2 commented Jan 10, 2018

turtles2 commented Apr 2, 2018

djaiss commented Apr 2, 2018

asbiin Apr 3, 2018

Choose a reason for hiding this comment

turtles2 Apr 4, 2018

Choose a reason for hiding this comment

asbiin Apr 4, 2018

Choose a reason for hiding this comment

asbiin commented Apr 4, 2018 • edited Loading

asbiin Apr 4, 2018

Choose a reason for hiding this comment

turtles2 Apr 5, 2018

Choose a reason for hiding this comment

turtles2 commented Apr 7, 2018 via email

asbiin commented Apr 7, 2018

djaiss commented Apr 8, 2018

turtles2 commented Apr 8, 2018 via email

github-actions bot commented Feb 11, 2021

turtles2 commented Jan 8, 2018 •

edited

Loading

asbiin commented Apr 4, 2018 •

edited

Loading