Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/App.vue
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
<template>
<v-app id="app">
<ErrorSnackBar/>
<Navbar></Navbar>
<router-view v-if="customLayout"/>
<v-container v-else class="full-height-content">
<router-view></router-view>
</v-container>
<div v-if="!failedToGetMissingPolicies" class="content">
<PolicyAcceptanceModal v-if="hasMissingPolicies"/>
<router-view v-if="customLayout"/>
<v-container v-else class="full-height-content">
<router-view></router-view>
</v-container>
</div>
<Foot :class="['padding-footer', { 'tall-footer': customLayout }]"></Foot>
<Interval
v-if="this.$store.getters.isLoggedIn && !this.$store.getters.currentUser.verified"
Expand All @@ -18,18 +22,30 @@
import Navbar from '@/components/Layout/Navbar'
import Foot from '@/components/Layout/Foot'
import Interval from '@/components/Util/Interval'
import PolicyAcceptanceModal from '@/components/Modals/PolicyAcceptanceModal'
import ErrorSnackBar from '@/components/Components/ErrorSnackBar'

export default {
name: 'App',
components: {
Navbar,
Foot,
Interval,
PolicyAcceptanceModal,
ErrorSnackBar,
},
computed: {
customLayout: function () {
return this.$route.meta.customLayout
},
hasMissingPolicies: function () {
return this.$route.meta.requiresAuth &&
!this.$route.meta.requiresAuth.excludeFromPolicyChecks &&
this.$store.getters.missingPolicies?.length

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't really get this bit...
hasMissingPolicies returns missingPolicies?.length, which is a number (or worst case undefined). Since this is used in v-if, is it less error-prone to return an explicit boolean (for example, (this.$store.getters.missingPolicies?.length ?? 0) > 0)?

},
failedToGetMissingPolicies: function () {
return this.$store.getters.failedToGetMissingPolicies
},
},
methods: {
checkVerified () {
Expand All @@ -48,10 +64,15 @@ export default {
.full-height-content {
height: 100%
}
.padding-footer.footer {
margin-top: 56px;
}
.tall-footer.footer {
height: 100%;
}
</style>

<style scoped>
.content {
position: relative;
height: 100%;
padding-bottom: 56px;
}
</style>
9 changes: 9 additions & 0 deletions src/backend/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ export const getCurrentPolicies = async () => {
const { data } = await axios.get('/v1/policies/current')
return data.items
}
export const getMissingPolicies = async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getMissingPolicies and getCurrentPolicies looks similar (same response shape), only the path differs.
You can consider writing getPolicies(path) or something like getPoliciesByStatus('missing' |'current') ti reduce repetition.

const { data } = await axios.get('/v1/policies/missing')
return data.items
}
export const acceptPolicies = async (policyIds) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

acceptPolicies doesn't return anything. It can return response.data or something

await axios.put('/v1/policy_acceptances', {
policy_ids: policyIds,
})
}

/* Wiki endpoints */
export const countWikis = async () => (await axios.get('/wiki/count')).data.data // TODO This doesn't seem to exist and not used?
Expand Down
45 changes: 8 additions & 37 deletions src/components/Cards/CreateAccount.vue
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,8 @@
<template v-slot:label v-if="policies.length">
<div>
I agree to the
<template v-for="(policy, idx) in policies">
<div :key="policy.metadata.policy_id" style="display: contents">
<span>{{ getPolicySeparator(idx) }}</span>
<v-tooltip bottom>
<template v-slot:activator="{ on }">
<a
target="_blank"
:href="policy.url"
@click.stop
v-on="on"
>{{ policy.name }}</a>
</template>
Opens in new window
</v-tooltip><span v-if="idx === policies.length - 1">.</span>
</div>
</template>
<PolicyList :policies="policies"/>
<span>.</span>
</div>
</template>
<template v-slot:label v-else>
Expand Down Expand Up @@ -111,13 +97,17 @@
</template>

<script>
import PolicyList from '../Components/PolicyList'

export default {
name: 'CreateAccountCard',
props: [
'title',
'buttonText',
],
components: {},
components: {
PolicyList,
},
computed: {
isLoggedIn: function () {
return this.$store.getters.isLoggedIn
Expand All @@ -140,14 +130,7 @@ export default {
this.checkCurrentLogin()

try {
this.policies = (await this.$api.getCurrentPolicies())
.map(
policy => ({
...policy,
name: policy.metadata.type.replaceAll('-', ' '),
url: `/${policy.metadata.type}/${policy.metadata.active_from}`,
}),
)
this.policies = await this.$api.getCurrentPolicies()
} catch (err) {
console.error(err)
// The policies array remains empty, so we fall back to the default terms of use
Expand All @@ -161,18 +144,6 @@ export default {
this.checkCurrentLogin()
},
methods: {
getPolicySeparator (idx) {
const count = this.policies.length
if (idx === 0) {
return ' '
} else if (count === 2) {
return ' and the '
} else if (idx === count - 1) {
return ', and the '
} else {
return ', the '
}
},
resetErrorState () {
this.hasError = false
this.error = []
Expand Down
46 changes: 46 additions & 0 deletions src/components/Components/ErrorSnackBar.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<template>
<v-snackbar
:value="show"
color="error"
:timeout="timeout"
@input="hide"
>
{{ message }}
<template
v-if="dismissible"
v-slot:action="{ attrs }"
>
<v-btn
icon
v-bind="attrs"
@click="hide"
>
<v-icon>mdi-close</v-icon>
</v-btn>
</template>
</v-snackbar>
</template>

<script>
export default {
computed: {
show: function () {
return this.$store.getters.hasLatestError
},
dismissible: function () {
return this.$store.getters.latestErrorDismissible
},
timeout: function () {
return this.dismissible ? 5000 : -1
},
message: function () {
return this.$store.getters.latestErrorMessage
},
},
methods: {
hide () {
this.$store.dispatch('clearLatestError')
},
},
}
</script>
63 changes: 63 additions & 0 deletions src/components/Components/PolicyList.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<template>
<div style="display: contents">
<div
v-for="(policy, idx) in policies"
:key="policy.metadata.policy_id"
style="display: contents"
>
<span>{{ getPolicySeparator(idx) }}</span>
<v-tooltip v-if="clickable" bottom>
<template v-slot:activator="{ on }">
<a
target="_blank"
:href="getPolicyUrl(policy)"
@click.stop
v-on="on"
>{{ getPolicyName(policy) }}</a>
</template>
Opens in new window
</v-tooltip>
<span v-else>
{{ getPolicyName(policy) }}
</span>
</div>
</div>
</template>

<script>
export default {
props: {
policies: {
type: Array,
},
article: {
type: String,
default: 'the ',
},
clickable: {
type: Boolean,
default: true,
},
},
methods: {
getPolicyName (policy) {
return policy.metadata.type.replaceAll('-', ' ')
},
getPolicyUrl (policy) {
return `/${policy.metadata.type}/${policy.metadata.active_from}`
},
getPolicySeparator (idx) {
const count = this.policies.length
if (idx === 0) {
return ' '
} else if (count === 2) {
return ` and ${this.article}`
} else if (idx === count - 1) {
return `, and ${this.article}`
} else {
return `, ${this.article}`
}
},
},
}
</script>
100 changes: 100 additions & 0 deletions src/components/Modals/PolicyAcceptanceModal.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
<template>
<div class="overlay">
<v-card max-width="580">
<v-card-title>
Updated policies
</v-card-title>
<v-card-text>
<p class="text-body-1">
Please accept the updated
<PolicyList
:policies="policies"
article=""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

article is empty, if the prop is optional, omit it

:clickable="false"
/>
to continue using Wikibase Cloud.
If you prefer not to keep your account, you can contact us to delete it.
</p>
<v-checkbox
v-model="accepted"
hide-details
class="mt-2"
:disabled="inFlight"
>
<template v-slot:label>
<div class="text-body-1">
I agree to the
<PolicyList :policies="policies"/>
<span>.</span>
</div>
</template>
</v-checkbox>
</v-card-text>
<v-card-actions>
<v-btn
text
color="primary"
@click="onContactForm"
:disabled="inFlight"
>
Contact Form
</v-btn>
<v-btn
color="primary"
:disabled="!accepted || inFlight"
@click="onAccept"
>
Continue
</v-btn>
</v-card-actions>
</v-card>
</div>
</template>

<script>
import PolicyList from '../Components/PolicyList'

export default {
components: {
PolicyList,
},
data () {
return {
accepted: false,
inFlight: false,
}
},
computed: {
policies () {
return this.$store.getters.missingPolicies
},
},
methods: {
onContactForm () {
this.$router.push('/contact')
},
async onAccept () {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

inFlight is set back to false only on the success path. If acceptPolicies throws/rejects or fails in general, inFlight will remain true and the UI will be stuck.
try/catch/finally can wrap the dispatch so inFlight is always reset

this.inFlight = true
const policyIds = Array.from(
this.policies,
policy => policy.metadata.policy_id,
)
await this.$store.dispatch('acceptPolicies', policyIds)
this.inFlight = false
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optional:
Add tests for the modal actions:

  1. clicking “Contact Form” navigates to /contact
  2. “Continue” is disabled until the checkbox is checked
  3. onAccept re-enables the UI even when acceptPolicies rejects (error path).

},
}
</script>

<style scoped>
.overlay {
background: rgba(0, 0, 0, 0.25);
position: absolute;
display: flex;
align-items: center;
justify-content: center;
inset: 0;
z-index: 10;
overflow: auto;
}
</style>
Loading
Loading